¿Duplicar constantes entre pruebas y código de producción?

20

¿Es bueno o malo duplicar datos entre pruebas y código real? Por ejemplo, supongamos que tengo una clase Python FooSaverque guarda archivos con nombres particulares en un directorio dado:

class FooSaver(object):
  def __init__(self, out_dir):
    self.out_dir = out_dir

  def _save_foo_named(self, type_, name):
    to_save = None
    if type_ == FOOTYPE_A:
      to_save = make_footype_a()
    elif type == FOOTYPE_B:
      to_save = make_footype_b()
    # etc, repeated
    with open(self.out_dir + name, "w") as f:
      f.write(str(to_save))

  def save_type_a(self):
    self._save_foo_named(a, "a.foo_file")

  def save_type_b(self):
    self._save_foo_named(b, "b.foo_file")

Ahora, en mi prueba, me gustaría asegurarme de que todos estos archivos fueron creados, así que quiero decir algo como esto:

foo = FooSaver("/tmp/special_name")
foo.save_type_a()
foo.save_type_b()

self.assertTrue(os.path.isfile("/tmp/special_name/a.foo_file"))
self.assertTrue(os.path.isfile("/tmp/special_name/b.foo_file"))

Aunque esto duplica los nombres de archivo en dos lugares, creo que es bueno: me obliga a escribir exactamente lo que espero que salga al otro lado, agrega una capa de protección contra errores tipográficos y, en general, me hace sentir seguro de que las cosas están funcionando exactamente como lo espero. Yo sé que si cambio a.foo_filea type_a.foo_fileen el futuro voy a tener que hacer un poco de búsqueda y reemplazo en mis pruebas, pero no creo que es demasiado grande de un acuerdo. Prefiero tener algunos falsos positivos si me olvido de actualizar la prueba a cambio de asegurarme de que mi comprensión del código y las pruebas estén sincronizadas.

Un compañero de trabajo piensa que esta duplicación es mala y me recomendó refactorizar ambos lados para hacer algo como esto:

class FooSaver(object):
  A_FILENAME = "a.foo_file"
  B_FILENAME = "b.foo_file"

  # as before...

  def save_type_a(self):
    self._save_foo_named(a, self.A_FILENAME)

  def save_type_b(self):
    self._save_foo_named(b, self.B_FILENAME)

y en la prueba:

self.assertTrue(os.path.isfile("/tmp/special_name/" + FooSaver.A_FILENAME))
self.assertTrue(os.path.isfile("/tmp/special_name/" + FooSaver.B_FILENAME))

No me gusta esto porque no me hace confiar en que el código está haciendo lo que esperaba --- Acabo de duplicar el out_dir + namepaso tanto en el lado de la producción como en el lado de la prueba. No descubrirá un error en mi comprensión de cómo +funciona en las cadenas, y no detectará errores tipográficos.

Por otro lado, es claramente menos frágil que escribir esas cadenas dos veces, y me parece un poco incorrecto duplicar datos en dos archivos como ese.

¿Hay un precedente claro aquí? ¿Está bien duplicar constantes en las pruebas y el código de producción, o es demasiado frágil?

Patrick Collins
fuente

Respuestas:

16

Creo que depende de lo que intentes probar, que va a lo que es el contrato de la clase.

Si el contrato de la clase es exactamente lo que FooSavergenera a.foo_filey b.foo_fileen una ubicación particular, entonces debe probarlo directamente, es decir, duplicar las constantes en las pruebas.

Sin embargo, si el contrato de la clase es que genera dos archivos en un área temporal, los nombres de cada uno se pueden cambiar fácilmente, especialmente en tiempo de ejecución, entonces debe probar de forma más genérica, probablemente usando constantes factorizadas de las pruebas.

Por lo tanto, debería discutir con su compañero de trabajo sobre la verdadera naturaleza y el contrato de la clase desde una perspectiva de diseño de dominio de nivel superior. Si no puede estar de acuerdo, diría que este es un problema del nivel de comprensión y abstracción de la propia clase, en lugar de probarlo.

También es razonable encontrar que el contrato de la clase cambia durante la refactorización, por ejemplo, para que su nivel de abstracción se eleve con el tiempo. Al principio, se trata de los dos archivos específicos en una ubicación temporal particular, pero con el tiempo, es posible que se justifique una abstracción adicional. En ese momento, cambie las pruebas para mantenerlas sincronizadas con el contrato de la clase. No hay necesidad de sobre construir el contrato de la clase de inmediato solo porque lo está probando (YAGNI).

Cuando el contrato de una clase no está bien definido, probarlo puede hacernos cuestionar la naturaleza de la clase, pero también lo haría usarlo. Diría que no debe actualizar el contrato de la clase solo porque lo está probando; debe actualizar el contrato de la clase por otros motivos, como que es una abstracción débil para el dominio y, de lo contrario, pruébelo como está.

Erik Eidt
fuente
4

Lo que sugirió @Erik, en términos de asegurarse de que tiene claro qué es lo que está probando, debería ser su primer punto de consideración.

Pero si su decisión lo lleva a la dirección de factorizar las constantes, eso deja la parte interesante de su pregunta (parafraseando) "¿Por qué debería cambiar las constantes duplicadas por el código duplicado?". (En referencia a donde hablas sobre "duplicar [ing] el paso out_dir + name").

Creo que (módulo comentarios de Erik) mayoría de los casos lo hacen en beneficio de la eliminación de duplicados constantes. Pero debe hacer esto de una manera que no duplique el código. En su ejemplo particular, esto es fácil. En lugar de tratar una ruta como cadenas "en bruto" en su código de producción, trate una ruta como una ruta. Esta es una forma más sólida de unir componentes de ruta que la concatenación de cadenas:

os.path.join(self.out_dir, name)

En su código de prueba, por otro lado, recomendaría algo como esto. Aquí, el énfasis está mostrando que tiene una ruta y está "conectando" un nombre de archivo de hoja:

self.assertTrue(os.path.isfile("/tmp/special_name/{0}".format(FooSaver.A_FILENAME)))

Es decir, mediante una selección más cuidadosa de elementos del lenguaje, puede evitar automáticamente la duplicación de código. (No todo el tiempo, pero muy a menudo en mi experiencia).

Michael Sorens
fuente
1

Estoy de acuerdo con la respuesta de Erik Eidt, pero hay una tercera opción: eliminar la constante en la prueba, por lo que la prueba pasa incluso si cambia el valor de la constante en el código de producción.

(vea tropezar una constante en Python unittest )

foo = FooSaver("/tmp/special_name")
foo.save_type_a()
foo.save_type_b()

with mock.patch.object(FooSaver, 'A_FILENAME', 'unique_to_your_test_a'):
  self.assertTrue(os.path.isfile("/tmp/special_name/unique_to_your_test_a"))
with mock.patch.object(FooSaver, 'B_FILENAME', 'unique_to_your_test_b'):
  self.assertTrue(os.path.isfile("/tmp/special_name/unique_to_your_test_b"))

Y cuando hago cosas como esta, generalmente me aseguro de hacer una prueba de cordura donde ejecuto las pruebas sin la withdeclaración y me aseguro de ver "'a.foo_file'! = 'Unique_to_your_test_a'", luego withvuelvo a poner la declaración en la prueba entonces pasa de nuevo.

alexanderbird
fuente