The Professor
Índice Índice del curso

Módulo 5 · Arquitectura y código mantenible

Lección 28 — Refactorizar sin romper nada

Refactor apoyado en pruebas: cambiar el interior sin mover el contrato.

Publicada
En esta lección
  1. Ejercicio 1 — Auditoría de la red
  2. Ejercicio 2 — Extracción guiada
  3. Ejercicio 3 — El experimento prohibido
  4. Ejercicio 4 — Contract guard
  5. Ejercicio 5 — Extracción del job
  6. Resumen del profesor

Ejercicio 1 — Auditoría de la red

Respuesta esperada (con huecos reales del curso hasta aquí): unitarias de la máquina de estados y resumen() (24/25), integración de la transacción con lock (10), contrato del problem+json del 409 (26) — y el hueco clásico: nadie prueba que el outbox se escribe DESPUÉS del commit de la reserva. El test que fija ese orden:

python
def test_outbox_solo_si_commit(self):
    with patch("core.db.transaction.atomic", side_effect=IntegrityError):
        with self.assertRaises(DomainError):
            reservar(self.user, self.event, ["A1"], clock=self.clock)
    self.assertFalse(OutboxEvent.objects.exists())   # rollback se llevó el evento

Si alguien publica antes del commit, este test rojo lo dice en 2 segundos.

Ejercicio 2 — Extracción guiada

El log esperado (los hashes son tuyos; lo que importa es la granularidad):

a1b2c3d test: caracterización del outbox tras commit (rojo→verde, sin tocar código)
e4f5a6b refactor: extraer _validar_asientos() (suite verde, 0 tests tocados)
b7c8d9e refactor: extraer _cobrar() tras el Protocol PaymentGateway (0 tests tocados)
c9d0e1f refactor: ReservacionService absorbe transacción+outbox (0 tests tocados)
d1e2f3a refactor: deprecar función suelta, delega en el servicio (1 test nuevo: deprecación)

La métrica del ejercicio: 0 tests de caracterización tocados en los 4 pasos (si tocaste alguno, el paso era dos). El paso 4: la función vieja muere cuando el grep de sus llamadas devuelve vacío UN sprint después; el test de deprecación (assertWarns(DeprecationWarning)) es el recordatorio vivo.

Ejercicio 3 — El experimento prohibido

Rojo esperado: los tests de contrato (26) que fijan el shape de la respuesta (el campo nuevo), el test del redondeo si existía (si no existía, el bug de pasada PASA desapercibido: esa es la lección — la mezcla se cuela cuando la red tiene agujeros, no porque el refactor sea malo), y posiblemente la E2E de compra completa (35). Número concreto: 3-5 rojos por una "mejora de pasada".

La versión bien hecha:

commit 1: test de regresión del redondeo (rojo) → fix mínimo (verde)   [bug]
commit 2: test del campo nuevo (rojo) → campo + nota de versión (14)    [feature]
commit 3-6: el refactor puro (0 tests tocados)                          [refactor]

Tres intenciones, tres commits, cada uno revertible sin arrastrar a los otros. El bisect de git (06) te lo agradecerá el día que algo se rompa en producción.

Ejercicio 4 — Contract guard

python
class ContractGuardTest(TestCase):
    def test_shape_de_reserva(self):
        res = self.client.get(f"/api/v1/reservations/{self.ref}")
        body = res.json()
        self.assertCountEqual(
            body.keys(),
            {"public_ref", "event", "seats", "total", "expires_at", "expires_in_seconds", "status"},
        )
        self.assertEqual(res.status_code, 200)

    def test_409_es_problem(self):
        res = self.client.post("/api/v1/reservations", {...})   # asiento ocupado
        assert_problem(self, res)
        self.assertEqual(res.json()["type"], BASE + "seat-unavailable")

Al renombrar seat_refs → seats, el test_shape_de_reserva falla en <2s (suite rápida) con un diff que dice exactamente qué campo faltaba. El valor del guard: convierte "cambiar el contrato" en algo que exige un commit consciente (el test se edita a propósito), no un accidente.

Ejercicio 5 — Extracción del job

python
# services/reservations.py
def expirar_reservas(clock: Clock) -> list[str]:
    vencidas = list(Reservation.objects.select_for_update(skip_locked=True).filter(
        status=Reservation.Status.ACTIVE, expires_at__lt=clock.now()
    ))
    refs = []
    for r in vencidas:
        r.expire()                      # transición de máquina de estados (00b/10)
        outbox.publish(ReservationExpired(ref=r.public_ref, user_id=r.user_id))
        refs.append(r.public_ref)
    return refs

# management command: SOLO transporte
class Command(BaseCommand):
    def handle(self, *args, **opts):
        refs = expirar_reservas(SystemClock())
        self.stdout.write(f"{len(refs)} reservas expiradas")

El test con FakeClock y skip_locked (10): si dos workers lo corren a la vez, no se pisan — el test de la 29 añadirá el worker Celery como nuevo transporte sin tocar la lógica. El mock del disparador verifica la delegación: patch("reservations.management.commands.expirar_reservas.expirar_reservas") y assert de llamada única + propagación de la excepción (el worker de la 29 necesita ver el fallo para reintentar).

Resumen del profesor

  • Refactor = interior cambia, observable no; la observable la define tu suite, y si tiene agujeros, la caracterización los tapa antes de mover nada.
  • Commits atómicos por intención: bug, feature y refactor nunca viajan juntos; el bisect y el revert lo exigen.
  • Extrae primero la lógica del mecanismo: el servicio queda estable y el transporte (cron → Celery → lo que venga) es un detalle del borde.

^ arriba