Duplikujesz stałe między testami a kodem produkcyjnym?

Czy powielanie danych między testami a prawdziwym kodem jest dobre czy złe? Załóżmy na przykład, że mam klasę Python, FooSaverktóra zapisuje pliki o określonych nazwach w danym katalogu:

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")

Teraz w moim teście chciałbym upewnić się, że wszystkie te pliki zostały utworzone, więc chcę powiedzieć coś takiego:

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"))

Mimo że powiela to nazwy plików w dwóch miejscach, myślę, że to dobrze: zmusza mnie do zapisania dokładnie tego, czego oczekuję od drugiego końca, dodaje warstwę ochrony przed literówkami i ogólnie daje mi pewność, że wszystko działa dokładnie tak, jak się spodziewam. Wiem, że jeśli zmienię a.foo_filesię type_a.foo_filew przyszłości, będę musiał przeprowadzić wyszukiwanie i zamianę w moich testach, ale nie sądzę, żeby to była zbyt wielka sprawa. Wolę mieć trochę fałszywych alarmów, jeśli zapomnę zaktualizować test w zamian za upewnienie się, że rozumiem kod i testy są zsynchronizowane.

Współpracownik uważa, że ​​to powielanie jest złe, i zalecił, aby po obu stronach zreformować coś takiego:

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)

oraz w teście:

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

Nie podoba mi się to, ponieważ nie daje mi to pewności, że kod działa zgodnie z oczekiwaniami --- właśnie zduplikowałem out_dir + namekrok po stronie produkcyjnej i testowej. Nie odkryje błędu w moim rozumieniu tego, jak +działa na ciągach, i nie złapie literówek.

Z drugiej strony jest to wyraźnie mniej kruche niż dwukrotne zapisywanie tych ciągów i wydaje mi się, że powielanie danych w dwóch takich plikach jest trochę niewłaściwe.

Czy jest tu wyraźny precedens? Czy można powielać stałe w testach i kodzie produkcyjnym, czy też jest zbyt kruche?

Myślę, że to zależy od tego, co próbujesz przetestować, co dotyczy kontraktu klasy.

Jeśli kontrakt klasy jest dokładnie taki, który FooSavergeneruje a.foo_filei znajduje się b.foo_filew konkretnej lokalizacji, powinieneś przetestować to bezpośrednio, tj. Zduplikować stałe w testach.

Jeśli jednak umowa klasy polega na tym, że generuje ona dwa pliki w obszarze tymczasowym, a nazwy każdego z nich można łatwo zmienić, szczególnie w czasie wykonywania, należy przeprowadzić bardziej ogólne testowanie, prawdopodobnie przy użyciu stałych uwzględnionych w testach.

Powinieneś więc spierać się ze współpracownikiem o prawdziwą naturę i kontrakt klasy z perspektywy projektowania domen na wyższym poziomie. Jeśli nie możesz się zgodzić, powiedziałbym, że jest to kwestia poziomu zrozumienia i abstrakcji samej klasy, a nie jej testowania.

Rozsądne jest również stwierdzenie zmiany kontraktu klasy podczas refaktoryzacji, na przykład w celu podniesienia poziomu abstrakcji w czasie. Na początku chodziło mi o dwa określone pliki w określonej lokalizacji tymczasowej, ale z czasem może się okazać, że dodatkowa abstrakcja jest uzasadniona. W takim przypadku zmień testy, aby były zsynchronizowane z umową klasy. Nie ma potrzeby od razu budować kontraktu klasy tylko dlatego, że go testujesz (YAGNI).

Kiedy kontrakt klasy nie jest dobrze zdefiniowany, testowanie go może sprawić, że zastanowimy się nad naturą klasy, ale i tak ją wykorzystamy. Powiedziałbym, że nie powinieneś ulepszać kontraktu klasy tylko dlatego, że go testujesz; powinieneś zaktualizować umowę klasy z innych powodów, takich jak słaba abstrakcja dla domeny, a jeśli nie, to przetestuj ją taką, jaka jest.

To, co sugerował @Erik - jeśli chodzi o upewnienie się, że testujesz - na pewno powinno być twoim pierwszym punktem do rozważenia.

Ale czy Twoja decyzja powinna poprowadzić Cię do kierunku wyliczenia stałych, co pozostawia interesującą część twojego pytania (parafrazowanie) „Dlaczego powinienem zamieniać powielające stałe na powielanie kodu?”. (W odniesieniu do miejsca, w którym mówisz o „duplikowaniu [katalogu] out_dir + nazwa kroku”.)

Wierzę, że (modulo komentarzy Erika) większość sytuacji zrobić korzyści z usuwania duplikatów stałe. Ale musisz to zrobić w sposób, który nie powiela kodu. W twoim przykładzie jest to łatwe. Zamiast traktować ścieżkę jako „nieprzetworzone” ciągi w kodzie produkcyjnym, traktuj ścieżkę jak ścieżkę. Jest to bardziej niezawodny sposób łączenia komponentów ścieżki niż konkatenacja łańcuchów:

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

Z drugiej strony w twoim kodzie testowym poleciłbym coś takiego. Tutaj nacisk kładzie się na to, że masz ścieżkę i „podłączasz” nazwę pliku liścia:

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

Oznacza to, że dzięki bardziej przemyślanemu wyborowi elementów językowych można automatycznie uniknąć powielania kodu. (Nie przez cały czas, ale bardzo często z mojego doświadczenia.)

Zgadzam się z odpowiedzią Erika Eidta, ale istnieje trzecia opcja: wytnij stałą w teście, więc test się powiedzie, nawet jeśli zmienisz wartość stałej w kodzie produkcyjnym.

(zobacz kasowanie stałej w 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"))

I robiąc takie rzeczy zwykle robię test poczytalności, w którym uruchamiam testy bez withinstrukcji i upewniam się, że widzę „a.foo_file”! = „Unique_to_your_test_a” ”, a następnie ponownie testuję withinstrukcję więc to mija ponownie.