V naší firemní kultuře v Topmonks máme takový zvyk: Každý týden dostane někdo pověření starat se o firemní sociální sítě. Tento týden to připadlo na mě. Vzhledem k tomu, že se sociálními sítěmi kromě Twitteru a LinkedInu si jinak netykám, připadal jsem si asi takto:
Ale mám rád výzvy a přemýšlel jsem, jak se takové výzvy zhostit. Slovy klasika z reklamy, mohli jsme vám nasdílet nějakou hypergeekovskou zábavu. Anebo fotku zátiší s notebookem z netradičních míst (zde střecha dílny na stavbě, nikde jinde v baráku totiž místo nebylo). Ale my jsme řekli ne! Možná to některé překvapí, ale v Topmonks taky kromě jiného pracujeme. Takže žádné 🍹 👙, ale do it hard way – rozhodl jsem se vyprázdnit buffer a spíchnout dohromady pár nedávných zkušeností. A když už je tedy to léto, tak to odlehčíme, tj. nebude to o produkčním kódu, ale jen o psaní testů
.Úkol, který jsem řešil, byl naimportovat z příchozího JSONu data do databáze, přesněji do 8 entitních typů vzájemně propojených všemi typy vztahů (many-to-many, one-to-many, many-to-one). Synchronizace dat mezi různými prostředími je důležitá operace a importní tool je pro ni stěžejní, proto jsme algoritmus nejen probírali na týmových mítincích, ale přirozeně ho chceme mít pokrytý vypovídajícími testy.
Před refactoringem
Prohlédněme si následující zjednodušenou a minimalizovanou ukázku. Vychází z reálné situace, kde v uvedené formě plně vyhovovala, dokud nebylo potřeba dělat větší zásahy do testovaného kódu.
Pro lepší kontext ještě pár technikálií: doménový model je reprezentován POJO objekty bez použití ORM frameworku (POJO neuchovávají asociace). Všechny databázové operace jdou pouze přes JDBC. ID objektů jsou typu String a fakticky obsahují UUID, které automaticky přiřadí DAO vrstva při vytvoření záznamu. Objekty mají kromě ID i unikátní klíč z business dat (název). DTO objekty reprezentující vstup importu obsahují – na rozdíl od POJO – i asociace. Testy jsou psané v JUnitu.
public class Cat {
String id;
String name;
Breed breed;
}
public class CatDto extends Cat {
List<Kitten> kittens;
}
public class Kitten {
String id;
String catId;
String name;
Color color;
}
public enum Breed {
BRITISH_SHORTHAIR,
MAINE_COON,
DOMESTIC_CAT;
}
public class CatImportServiceTest {
@Inject
private CatImportService catImportService;
@Inject
private CatDao catDao;
private Cat existingCatWithThreeKittens;
private CatDto receivedCat;
@Before
public void setup() {
jdbcTemplate.update("delete from kittens");
jdbcTemplate.update("delete from cat");
Cat c1 = new Cat();
c1.setName("Felix");
c1.setBreed(Breed.DOMESTIC_CAT);
existingCatWithThreeKittens = catDao.create(c1);
List<Kitten> kittens = new ArrayList<>();
Kitten kitten1 = new Kitten();
kitten1.setId("10");
kitten1.setCatId(existingCatWithThreeKittens.getId());
kitten1.setName("Tiger");
kitten1.setColor(Color.GRAY);
kittens.add(kitten1);
Kitten kitten2 = new Kitten();
kitten2.setId("20");
kitten2.setCatId(existingCatWithThreeKittens.getId());
kitten2.setName("n1");
kitten2.setColor(Color.BLACK);
kittens.add(kitten2);
Kitten kitten3 = new Kitten();
kitten3.setId("30");
kitten3.setCatId(existingCatWithThreeKittens.getId());
kitten3.setName("dummy");
kitten3.setColor(Color.WHITE);
kittens.add(kitten3);
catDao.createKittens(existingCatWithThreeKittens.getId(), kittens);
}
@Test
public void importNewCatCreatesTheCat() {
receivedCat = new CatDto();
receivedCat.setId("2");
receivedCat.setName("Charlie");
final Breed expectedBreed = Breed.MAINE_COON;
receivedCat.setBreed(expectedBreed);
List<Cat> receivedCats = new ArrayList<>();
receivedCats.add(receivedCat);
doImport(receivedCats);
Cat importedCat = catDao.getByName("Charlie");
assertEquals(expectedBreed, importedCat.getBreed())
assertEquals(2, catDao.getAll().size())
}
@Test
public void importExistingCatReplacesHerKittens() {
receivedCat = new CatDto();
String receivedCatId = "2";
receivedCat.setId(receivedCatId);
receivedCat.setName("Felix");
receivedCat.setBreed(Breed.MAINE_COON);
List<Kitten> kittens = new ArrayList<>();
Kitten kitten1 = new Kitten();
kitten1.setId("30");
kitten1.setCatId(existingCatWithThreeKittens.getId());
kitten1.setName("some");
kitten1.setColor(Color.BROWN);
kittens.add(kitten1);
Kitten kitten2 = new Kitten();
kitten2.setId("40");
kitten2.setCatId(existingCatWithThreeKittens.getId());
kitten2.setName("any");
kitten2.setColor(Color.YELLOW);
kittens.add(kitten2);
receivedCat.setKittens(kittens);
List<Cat> receivedCats = new ArrayList<>();
receivedCats.add(receivedCat);
doImport(receivedCats);
Cat catInDb = catDao.getByName("Felix");
List<Kitten> kittensInDb = catDao.getKittens(catInDb);
assertEquals(3, kittensInDb.size())
assertEquals("n1", kittens.get(0).getName());
assertEquals(Color.BLACK, kittens.get(0).getColor());
assertEquals("some", kittens.get(1).getName());
assertEquals(Color.BROWN, kittens.get(1).getColor());
assertEquals("any", kittens.get(2).getName());
assertEquals(Color.YELLOW, kittens.get(2).getColor());
}
void doImport(List<CatDto> receivedCats) {
catImportService.importAllCats(receivedCats);
}
void doImport(CatDto receivedCat) {
catImportService.importAllCats(Collections.singletonList(receivedCat));
}
}
Které vlastnosti takového testu Vám nejvíc vadí? Tady je můj seznam:
Refactorujeme
Společný setup
Možná nejkontroverznější téma hned na začátek.S masivním rozšířením JUnitu jsme si zvykli, že testovací třída má řadu @Test metod a pak zpravidla jednu setupovací @Before metodu (pozn.: těch může samozřejmě technicky být i víc, ale neznamená to různý setup pro různé testy a není mezi nimi ani specifikováno pořadí). Aby bylo jasno, samotnou testovací pattern Arrange-Act-Assert považuji za dobrou. Mám ale dojem, že někdy máme sklon dávat do @Before části věci, které patří do části @Test. Podpořme to ještě důvěryhodným argumentem o DRY, proti němuž nikdo nic nenamítne, a vznikne obvykle třída, kde se v @Before připraví univerzální setup, na který pak začnou z různých směrů útočit testovací metody.
Problém tohoto přístupu je, že není vhodný pro úlohy typu import. Logika importu je funkcí nejen vstupních dat, ale i aktuálního setupu databáze. Setup databáze je tedy přirozenou součástí @Test metod, do @Before patří skutečně pouze základní společné věci jako smazání všech tabulek nebo autorizace. Domnívám se, že v tomto případě není univerzální setup vhodný vzhledem k rozmanitosti testovacích dat – buď musí testovací metoda ještě něco přidat a setup se tříští na dvě místa, nebo odmazat, případně nevyužít, a setup je tak nepřehledný. (Např. kočka, která se v ukázce nasetupuje, je pro test importNewCatCreatesTheCat nejen zbytečná, ale dokonce ho i zamlžuje assertem na celkovou velikost, který s existencí předpřipravené kočky musí počítat.)
Jak z toho ven?
Pokud se setupy pro jednotlivé testy dají seskupit do malého počtu typových případů, pak je lze rozdělit do několika testovacích tříd (případně, pokud jeden setup navazuje na jiný, tuto návaznost vyjádřit dědičností tříd s provoláním super.setup()). Osobně se ale domnívám, že cena za takové zavedení principu DRY je ale vyšší než se zdá a vzhledem k zesložitění v oblasti počtu tříd a hierarchie se to nevyplatí.
Řešením je změnit přístup. Setup přes @Before je push-přístup typický pro framework, kdy přizpůsobujeme návrh vlastního kódu hookům, které navrhl autor frameworku. Zaměřme se raději na to, aby každá testovací metoda si nasetupovala jen to, co opravdu potřebuje (pull-přístup), a jak to udělat co nejsrozumitelněji. O tom budou i zbylé tipy.
Tvorba testovacích objektů
JavaBeans specifikace vznikla v roce 1997, kdy si snad nikdo nepředstavoval Javu jinak než jako imperativní jazyk. Klasická tvorba POJO objektu pomocí série z příkazu new a několika setterů sice z pohledu různých builderů a fluent zápisů působí zastarale (a v některých ohledech oprávněně – např. nutí k samostatné deklaraci a vymýšlení jména pro objekt jen proto, že je potřeba ho nasetovat), má však svoje místo i dnes. Není to pouze díky své jednoduchosti, ale v prostředí testů zejména i díky tomu, že je možno si vybrat, které properties nastavíme. Pro maximální jednoduchost a čitelnost testu by mělo být zásadní, aby se nastavovaly jen ty properties, které hrají roli v otestování dané funkcionality.Pokud klasický způsob tvorby POJO objektu překáží (např. pro nepoužitelnost ve fluent zápisu) a sada nastavovaných properties je jedna a táž, pak preferuji použití factory metody (buď v testu nebo přímo v POJO třídě). Pokud je sad víc, ale malý únosný počet, pak se ještě dá spokojit s přetížením factory metod. Tady ovšem pozor, aby se přetěžování nezvrhlo co do množství přetížení nebo dodržení best practices. Ve zcela obecném případě, tj. chceme-li fluent zápis a přitom flexibilní volbu nastavovaných properties, pak zbývá jen builder pattern.
Osobně používám fluent zápis rád, považuji proto jak přetěžované metody, tak builder pattern za dobrou pomůcku. Osvědčila se konvence začínat factory metody prefixem new... (na rozdíl od názvosloví create..., které je vyhrazeno pouze pro tvorbu v databázi na úrovni DAO), což pak umožní one-linery na způsob create(newFoo()) s čitelností na úrovni anglické věty.
Tvorba testovacích kolekcí
Podobně jako POJO objekty, i kolekce můžeme tvořit klasicky i pomocí jednořádkového zápisu. Na rozdíl od POJO objektů je zde ale lepší podpora, ať už pomocí vestavěného Arrays.asList() či s podporou Guava metody ImmutableList.of().Pokud se testuje funkcionalita, která má na vstupu kolekci, jistě by nemělo zůstat u toho, aby veškeré testy pracovaly pouze s jednoprvkovou kolekcí. Nicméně pro některé testy kolekce o jediném prvku stačí a pak je zbytečné testy komplikovat. V tom případě se nabízí přetížit příslušné metody, aby akceptovaly jak kolekci, tak jediný prvek. Já to ale považuji za past a zbytečný syntaktický cukr, který pouze snižuje ortogonalitu (rada #40 v Effective Java).
V uvedené ukázce tedy preferuji použít jen metodu doImport(List). Volání s jediným prvkem nechť explicitně uvedou Collections.singletonList() a bude to každému jasné. Kromě toho takto redukovaná signatura mnohem lépe škáluje, protože při n různých Listech na vstupu stačí pořád jediná metoda, zatímco při přetížení každého argumentu bychom potřebovali n! metod 2n metod.
Samovypovídající vstupní data
Dalším místem, kde lze zlepšit čitelnost testů doslova zadarmo, je volba testovacích dat. Pokud z hlediska testu dává smysl naplnit stringový field libovolným řetězcem, pak namísto náhodného nicneříkajícího řetězce preferuji řetězec, který přináší přidanou dokumentační hodnotu. Testuji přidání nové kočky? Raději než hledat v kalendáři kočičích jmen, ji pojmenuju Newcat. Testuji logiku zaměřenou na situaci, kdy osoba má stejné jméno a příjmení? Raději než Franta Franta ji pojmenuju Same Same. Testuji, zda seznam řetězců neobsahuje hodnotu? Nastražím data testu tak, abych mohl napsat assertFalse(list.contains("nonexisting")). Testuju CRUD operace nad koťaty? Pojmenuju si příslušná koťata Created, Deleted, Changed, Unchanged. A tak dále.I u číselných typů se lze přiblížit podobnému samodokumentačnímu efektu, pokud si zvolená čísla s sebou nesou nějakou známou (ať pozitivní či negativní) konotaci. U typů s omezenou doménou jako např. enumy nebo boolean uvedený postup použít nelze, nicméně i tam je lepší zvolit hodnoty tak, aby celkově test vypadal konzistentně.
A když už naše testovací data tak pěkně vypadají a je z nich patrné, že to nejsou náhodně vymyšlené hodnoty, pak by bylo kontraproduktivní to vysušovat a ukládat tyto hodnoty do lokálních proměnných. Ano, z hlediska formální správnosti není nic přesnějšího než v metodě importNewCatCreatesTheCat uložit očekávané plemeno kočky do finální proměnné a proti ní pak porovnávat skutečnou hodnotu. Jenže to stopování výskytů proměnné při čtení testu už mi přijde stejně namáhavé jako stopování výskytů konstanty, že mi to za ten řádek navíc a vymýšlení názvu nestojí.
Vlastní asserty
Podobně jako si můžeme zčitelnit tvorbu objektů, můžeme zčitelnit asserty. Není nutné se po řádkách ptát přes assertEquals na jednotlivé fieldy: máme JUnit Matchery nebo můžeme vytknout opakující se věci do šikovně pojmenované metody. Obzvlášť ve spojení s lambda funkcemi se dá dosáhnout poměrně srozumitelného zápisu na malém počtu řádků.Trochu offtopic: v uvedené ukázce se test na koťata v databázi spoléhá na seřazení podle ID. To dělá test fragilním (a pokud je řazení myšleno jako součást testu, pak by měla existovat vlastní testovací metoda pouze na něj). Vylepšené implementaci pomocí predikátu už na pořadí nezáleží, stále však netestuje, že predikát našel pokaždé jiný prvek. To se pojistí teprve až testem na velikost množiny, do které se objekty nahází (za předpokladu, že mají na ID postavenou metodu equals).
Ostatní, menší smells
Je to asi věc vkusu, ale používám statické importy, kde můžu, obzvlášť u enum konstant a factory metod pro kolekce jako emptyList() a singletonList() (pozn.: u emptyList() toto před verzí Javy 8 nebylo možné vzhledem ke slabé typové inferenci a nutnosti použít explicitní zápis Collections.<Foo>emptyList()).Setup na testovacích metodách by neměl zacházet do nižší úrovně abstrakce, tj. na úroveň SQL, ale měl by zůstat na úrovni DAO.
Po refactoringu
public class CatImportServiceTest {
@Inject
private CatImportService catImportService;
@Inject
private CatDao catDao;
@Before
public void setup() {
catDao.deleteAllKittens();
catDao.deleteAllCats();
}
@Test
public void importNewCatCreatesTheCat() {
CatDto receivedCat = newCat("1","Newcat",MAINE_COON);
catImportService.importAllCats(singletonList(receivedCat));
Cat importedCat = catDao.getByName("Newcat");
assertEquals(MAINE_COON, importedCat.getBreed())
assertEquals(1, catDao.getAll().size())
}
@Test
public void importExistingCatReplacesHerKittens() {
catDao.create(newCat("1","Existing",DOMESTIC_CAT));
catDao.createKittens("1", ImmutableList.of(
newKitten("10","1","Deleted",RED),
newKitten("20","1","Unchanged",WHITE),
newKitten("30","1","Changed",BLACK)
));
CatDto receivedCat = newCat("1","Existing",DOMESTIC_CAT)
.withKittens(ImmutableList.of(
newKitten("20","1","Unchanged",WHITE),
newKitten("30","1","Changed",BROWN),
newKitten("40","1","Created",GREEN)
));
catImportService.importAllCats(singletonList(receivedCat));
Cat catInDb = catDao.getByName("Existing");
List<Kitten> kittensInDb = catDao.getKittens(catInDb.getId());
assertEquals(3, ImmutableSet.copyOf(kittensInDb).size());
assertKittenExists(kittensInDb, k -> "Unchanged".equals(k.getName()) && WHITE == k.getColor());
assertKittenExists(kittensInDb, k -> "Changed".equals(k.getName()) && BROWN == k.getColor());
assertKittenExists(kittensInDb, k -> "Created".equals(k.getName()) && BLACK == k.getColor());
}
protected static void assertKittenExists(List<Kitten> actualKittens, Predicate<Kitten> condition) {
assertTrue(actualKittens.stream().filter(predicate).count() == 1);
}
}
Shrnutí
Nechávám na čtenáři posouzení, zda popisované refactoringy čitelnost kódu vylepšily. Docela dost je možné, že pohled na to, co je čitelný kód, má každý trochu jinde a také kvůli této validaci jsem blogpost psal. Nicméně chtěl jsem demonstrovat, že ne všechny poučky, které bychom se neodvážili porušit v produkčním kódu, musí nutně pomáhat i v prostředí testů. Svoji roli tu hrají i vlastnosti Javy jako silné typování a některé slabiny syntaxe.A jestli se Vám článek líbil, vytrollil, popřípadě nechal jiné silné emoce, lajkujte, retweetujte, přijďte k nám nezávazně na pokec na snídani nebo rovnou rozšiřte naše řady :-). Hezké léto!
