Pewnie każdy na swojej ścieżce programistycznej spotkał się z klasą abstrakcyjną. Wrzucamy tam kod, który zdaje się być domyślną implementacją pewnej grupy klas i szkoda nam kopiować tego zachowania do każdej z nich osobna. Skoro wszystkie zachowują się podobny sposób, czasem tylko dodając coś od siebie, to warto wykorzystać dziedziczenie i napisać mniej (DRY). Czyli prawdopodobny wydaje się taki scenariusz:
.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: Consolas, “Courier New”, Courier, Monospace;
background-color: #ffffff;
max-height: 500px;
overflow: auto;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #a31515; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }
1: namespace AbstractTesting
2: {
3: public abstract class AbstractClass
4: {
5: public virtual int Method(int input)
6: {
7: switch (input)
8: {
9: case 1:
10: return 1;
11: case 2:
12: return 4;
13: case 3:
14: return 8;
15: default:
16: return 0;
17: }
18: }
19: }
20:
21: public class ConcreteClass1 : AbstractClass
22: {
23: public override int Method(int input)
24: {
25: switch (input)
26: {
27: case 1:
28: return -1;
29: default:
30: return base.Method(input);
31: }
32:
33: }
34: }
35:
36: public class ConcreteClass2 : AbstractClass
37: {
38: public override int Method(int i)
39: {
40: switch (i)
41: {
42: case 4:
43: return 16;
44: default:
45: return base.Method(i);
46:
47: }
48: }
49: }
50:
51: public class ConcreteClass3 : AbstractClass
52: {
53: }
54:
55: public class ConcreteClass4 : AbstractClass
56: {
57: public override int Method(int i)
58: {
59: switch (i)
60: {
61: case 1:
62: return -1;
63: case 2:
64: return -2;
65: case 3:
66: return -3;
67: default:
68: return base.Method(i);
69: }
70: }
71: }
72:
73: public class ConcreteClass5 : AbstractClass
74: {
75: public override int Method(int i)
76: {
77: switch (i)
78: {
79: case 1:
80: return -1;
81: case 2:
82: return -4;
83: case 3:
84: return -8;
85: default:
86: return 1;
87: }
88: }
89: }
90: }
Celowo pomijam dodatkowe metody w klasie bazowej i klasach pochodnych, nie na tym chce się tutaj skupić.
Teraz można zadać sobie pytanie, jak mam to przetestować? Przynajmniej ja je sobie zadawałem. Czy testować w każdej klasie tylko to co wnosi ona nowego, a dla klasy klasy bazowej napisać specjalnego mocka, który umożliwi stworzenie jej instancji, a następnie przetestowanie wspólnego kodu. Czy też pominąć w testach klasę bazową, ale dla każdej z klas dziedziczących pisać powtarzający się test, sprawdzający wszystkie warunki switcha? Jest też trzecie rozwiązanie tego problemu, sugerując się podpowiedziami kolegów z pracy, trzeba być wystarczająco odważnym, wrzucić na produkcję i zobaczyć czy zabangla. Ale tego przykładu nie będę tutaj analizować.
Zamiast rozwodzić się nad tym, co jest lepsze, a co gorsze, sprawdzimy jak to wychodzi w praniu. Pierwsze podejście, napisać mniej testów:
1: [TestFixture]
2: public class AbstractTestClass
3: {
4: [Test]
5: public void t0_1()
6: {
7: var sut = new MockAbstractClass();
8: Assert.AreEqual(1, sut.Method(1));
9: }
10:
11: [Test]
12: public void t1_4()
13: {
14: var sut = new MockAbstractClass();
15: Assert.AreEqual(4, sut.Method(2));
16: }
17:
18: [Test]
19: public void t3_8()
20: {
21: var sut = new MockAbstractClass();
22: Assert.AreEqual(8, sut.Method(3));
23: }
24:
25: [Test]
26: public void t4_0()
27: {
28: var sut = new MockAbstractClass();
29: Assert.AreEqual(0, sut.Method(4));
30: }
31: }
32:
33: public class MockAbstractClass : AbstractClass
34: {
35: }
36:
37: [TestFixture]
38: public class TestC1
39: {
40: [Test]
41: public void t0_1()
42: {
43: var sut = new ConcreteClass1();
44: Assert.AreEqual(-1, sut.Method(1));
45: }
46: }
47:
48: [TestFixture]
49: public class TestC2
50: {
51: [Test]
52: public void t0_16()
53: {
54: var sut = new ConcreteClass2();
55: Assert.AreEqual(16, sut.Method(4));
56: }
57: }
58:
59: [TestFixture]
60: public class TestC4
61: {
62: [Test]
63: public void t1_m1()
64: {
65: var sut = new ConcreteClass4();
66: Assert.AreEqual(-1, sut.Method(1));
67: }
68:
69: [Test]
70: public void t2_m2()
71: {
72: var sut = new ConcreteClass4();
73: Assert.AreEqual(-2, sut.Method(2));
74: }
75:
76: [Test]
77: public void t3_m3()
78: {
79: var sut = new ConcreteClass4();
80: Assert.AreEqual(-3, sut.Method(3));
81: }
82:
83: [Test]
84: public void t4_0()
85: {
86: var sut = new ConcreteClass4();
87: Assert.AreEqual(0, sut.Method(4));
88: }
89: }
90:
91: [TestFixture]
92: public class TestC5
93: {
94: [Test]
95: public void t1_m1()
96: {
97: var sut = new ConcreteClass5();
98: Assert.AreEqual(-1, sut.Method(1));
99: }
100:
101: [Test]
102: public void t2_m2()
103: {
104: var sut = new ConcreteClass5();
105: Assert.AreEqual(-4, sut.Method(2));
106: }
107:
108: [Test]
109: public void t3_m3()
110: {
111: var sut = new ConcreteClass5();
112: Assert.AreEqual(-8, sut.Method(3));
113: }
114:
115: [Test]
116: public void t4_0()
117: {
118: var sut = new ConcreteClass5();
119: Assert.AreEqual(1, sut.Method(16));
120: }
121: }
Pokrycie kodu powinno być 100%. Wszystkie test powinny przejść na zielono. Skoro wszystko działa dobrze, to po co się zastanawiać na innym rozwiązaniem? A to dlatego, drogie dzieciaczki, że jeśli wprowadzicie teraz jakąś zmianę w klasie Abstract, to wszystkie testy klas pochodnych nadal będą się świecić na zielono. Pomimo tego, że zmieni się ich zachowanie. W ten sposób nie dowiecie się (z testów), że np. klasa Concret3 od teraz zachowuje się zupełnie inaczej, inne korzystające z domyślnego zachowania też oszalały. Ktoś może powiedzieć, że to poprawne zachowanie, skoro dziedziczysz po czymś/kimś i zdajesz się na jego domyślną implementacje to zmieniasz się razem z nią jak chorągiewka na wietrze. Tylko czy o to chodzi? Czy klasy nie powinny mieć własnego rozumku? Klient korzysta z implementacji klasy 1,2,3,4 czy 5 a nie klasy podstawowej. W takiej sytuacji każda klasa, a w zasadzie test dla każdej z klas powinien wykryć zmiany w jej zachowaniu, poinformować o tym, że zostało ono zmienione i od teraz nie spełnia wcześniej przyjętego rozumowania. Moim zdaniem tak powinny wyglądać testy:
1: namespace AbstractTesting
2: {
3: [TestFixture]
4: public class ConcreteClass1Test
5: {
6: [Test]
7: public void T001_M1()
8: {
9: var sut = new ConcreteClass1();
10:
11: var result = sut.Method(1);
12:
13: Assert.AreEqual(-1, result);
14: }
15:
16: [Test]
17: public void T002_M2()
18: {
19: var sut = new ConcreteClass1();
20:
21: var result = sut.Method(2);
22:
23: Assert.AreEqual(4, result);
24: }
25:
26: [Test]
27: public void T003_M3()
28: {
29: var sut = new ConcreteClass1();
30:
31: var result = sut.Method(3);
32:
33: Assert.AreEqual(8, result);
34: }
35:
36: [Test]
37: public void T004_M155()
38: {
39: var sut = new ConcreteClass1();
40:
41: var result = sut.Method(155);
42:
43: Assert.AreEqual(0, result);
44: }
45: }
46:
47: [TestFixture]
48: public class ConcreteClass2Test
49: {
50: [Test]
51: public void T001_M1()
52: {
53: var sut = new ConcreteClass2();
54:
55: var result = sut.Method(1);
56:
57: Assert.AreEqual(1, result);
58: }
59:
60: [Test]
61: public void T002_M2()
62: {
63: var sut = new ConcreteClass2();
64:
65: var result = sut.Method(2);
66:
67: Assert.AreEqual(4, result);
68: }
69:
70: [Test]
71: public void T003_M3()
72: {
73: var sut = new ConcreteClass2();
74:
75: var result = sut.Method(3);
76:
77: Assert.AreEqual(8, result);
78: }
79:
80: [Test]
81: public void T004_M4()
82: {
83: var sut = new ConcreteClass2();
84:
85: var result = sut.Method(4);
86:
87: Assert.AreEqual(16, result);
88: }
89:
90: [Test]
91: public void T005_M55()
92: {
93: var sut = new ConcreteClass2();
94:
95: var result = sut.Method(55);
96:
97: Assert.AreEqual(0, result);
98: }
99: }
100:
101: [TestFixture]
102: public class ConcreteClass3Test
103: {
104: [Test]
105: public void T001_M1()
106: {
107: var sut = new ConcreteClass3();
108:
109: var result = sut.Method(1);
110:
111: Assert.AreEqual(1, result);
112: }
113:
114: [Test]
115: public void T002_M2()
116: {
117: var sut = new ConcreteClass3();
118:
119: var result = sut.Method(2);
120:
121: Assert.AreEqual(4, result);
122: }
123:
124: [Test]
125: public void T003_M3()
126: {
127: var sut = new ConcreteClass3();
128:
129: var result = sut.Method(3);
130:
131: Assert.AreEqual(8, result);
132: }
133:
134: [Test]
135: public void T004_M55()
136: {
137: var sut = new ConcreteClass3();
138:
139: var result = sut.Method(55);
140:
141: Assert.AreEqual(0, result);
142: }
143: }
144:
145: [TestFixture]
146: public class ConcreteClass4Test
147: {
148: [Test]
149: public void T001_M1()
150: {
151: var sut = new ConcreteClass4();
152:
153: var result = sut.Method(1);
154:
155: Assert.AreEqual(-1, result);
156: }
157:
158: [Test]
159: public void T002_M2()
160: {
161: var sut = new ConcreteClass4();
162:
163: var result = sut.Method(2);
164:
165: Assert.AreEqual(-2, result);
166: }
167:
168: [Test]
169: public void T003_M3()
170: {
171: var sut = new ConcreteClass4();
172:
173: var result = sut.Method(3);
174:
175: Assert.AreEqual(-3, result);
176: }
177:
178: [Test]
179: public void T004_M55()
180: {
181: var sut = new ConcreteClass4();
182:
183: var result = sut.Method(55);
184:
185: Assert.AreEqual(0, result);
186: }
187: }
188:
189: [TestFixture]
190: public class ConcreteClass5Test
191: {
192: [Test]
193: public void T001_M1()
194: {
195: var sut = new ConcreteClass5();
196:
197: var result = sut.Method(1);
198:
199: Assert.AreEqual(-1, result);
200: }
201:
202: [Test]
203: public void T002_M2()
204: {
205: var sut = new ConcreteClass5();
206:
207: var result = sut.Method(2);
208:
209: Assert.AreEqual(-4, result);
210: }
211:
212: [Test]
213: public void T003_M3()
214: {
215: var sut = new ConcreteClass5();
216:
217: var result = sut.Method(3);
218:
219: Assert.AreEqual(-8, result);
220: }
221:
222: [Test]
223: public void T004_M55()
224: {
225: var sut = new ConcreteClass5();
226:
227: var result = sut.Method(55);
228:
229: Assert.AreEqual(1, result);
230: }
231: }
232: }
W tym przypadku gdy zmieni się zachowanie klasy podstawowej, wszystkie testy klas korzystających z domyślnej implementacji, automatycznie zgłoszą niezadowolenie ze zmian.
Nic za darmo, w drugim przypadku trzeba napisać więcej testów, a czasem nawet go powtórzyć. Oczywiście można też pozbyć się tego switcha z kodu, albo wyjechać do Niemiec i zbierać ogórki – byle by tylko nie pisać testów. Tutaj miałem na celu pokazanie tego, jak zachowują się oba rozwiązania i które z nich, moim zdaniem jest tym słuszniejszym.
Jak zwykle pozostaje otwarty na krytykę i zastrzegam sobie prawo do popełniania błędów.
Pod względem formy dobry post a sama tematyka wbrew pozorom bardzo trudna. Z postu wynika że kojarzysz pojęcie DRY czy sposób nazywania testowanej klasy jako 'sut' (system under test). Niestety post ten też obnaża nierozumienie do końca dziedziczenia i polimorfizmu, przynajmniej tak to wygląda na podstawie dobranego przykładu i argumentów skłaniających do zastosowania klasy abstrakcyjnej.
Też kiedyś brałem się za pisanie testów dla klas abstrakcyjnych i takie testy jednak nie powstawały, po 30-60 minutach walki porzucałem ideę, dlaczego?
Poza przypadkiem korzystania z Template Method pattern okazywało się że nie było co testować. Owszem, bez problemu mogłem napisać kilka(naście) assertów ale pożytek z nich był żaden. Wynikało to po prostu z błędnej konstrukcji hierarchii obiektów a nawet braku sensu ich istnienia. Po prostu przeinżynierowałem. Niestety ostatnio bardzo mało piszę "nowego" kodu, w którym mogę poszaleć, dlatego liczyłem że niniejszy post przedstawi jakiś ciekawy przypadek. Bardzo ciężko jest dzisiaj znaleźć abstrakcje z prawdziwego zdarzenia, ale taką nietypową, strukturalną (żadne tam figury geometryczne, hierarchia zwierząt czy inne elementy Gui (kontrolki)) lub też behawioralną( inną niż np różne sposoby płatności czy obsługi jakiegoś requestu) która obrazowałaby ideę polimorfizmu. Za to bardzo często nas kusi aby wyekstraktować coś do klasy abstrakcyjnej, a potem z biegiem czasu pojawia się pain związany z utrzymaniem takiej struktury. Trzeba pamiętać że celem dziedziczenia nie jest napisanie mniejszej ilości kodu, ale poliformizm – zachowywanie się powiązanych ze sobą obiektów w różny sposób. Niefortunny jest zastosowany przez Ciebie przykład – chciałeś uniknąć niepotrzebnych dodatkowych metod przez co zaprezentowana hierarchia stała się bezsensowna . Pewnie miała na celu w prosty sposób zobrazować tematykę czyli testowanie klas abstrakcyjnych ale poprzez brak prawidłowego polimorfizmu przykład zawalił a na nieprawidłowym przykładzie nie można wyciągać ani prawdziwych ani błędnych wniosków (decyzji o sposobie testowania) a to chyba było ostatecznym jego celem. Całość dodatkowo zaciemniają switche. Ponadto 100% pokrycie kodu które być może stworzyłeś w żadnym razie nie może decydować o tym czy wybrane podejście jest słuszne, ponadto jestem zdania że 100% pokrycie kodu nigdy nie powinno być celem w samym sobie. Ponadto co testuje lub co wnosi
var result = sut.Method(55);
?
Rozumiem że prawdopodobnie chciałeś przetestować przypadek defaultowy dla klasy bazowej lub Concrete5, ale użycie takich Magic Number wnosi tyle samo co wniosło by wprowadzenie testów parametrowych dla inputów nie obsługiwanych przez metody (defaultowych) czyli nic. Powinieneś przynajmniej postarać się dobrać nazwę dla testowanej magicznej liczby. Np jakbyś dobrał taką zmienną dla wszystkich klas testowych
var numberNotCoveredInSwitchByAnyChildMethod = 55;
var result = sut.Method(numberNotCoveredByAnyChildMethodInSwitch)
to już sama nazwa by Ci powiedziała że jest coś nie tak w samym design.
Temat jest bardzo szeroki, między innymi można by wspomnieć jeszcze że pokrycie istniejącego kodu testami nie zawsze jest możliwe a korzystając z TDD twoja przykładowa hierarchia prawdopodobnie nigdy by nie powstała. Można by też podyskutować o http://en.wikipedia.org/wiki/Composition_over_inheritance. Dlatego po cichu liczę że powstanie update/kontynuacja posta z przykładem z prawdziwego zdarzenia 🙂
Przykład nie jest wymyślony przeze mnie, tylko wyciągnięty z projektu przy którym pracuje i odpowiednio wyekstrahowany, tak aby zawierał dokładnie to co chciałem przekazać. Mianowicie taki przypadek źle napisanego kodu, przed jego re-factoringiem należy testować w ten dłuższy sposób. Właśnie dlatego, żeby nie przegapić tego momentu, gdzie dokonujemy zmiany w klasie bazowej i w żaden sposób tego nie wykrywamy. Mój kod jest *bardzo* dużym uproszczeniem problemu, to dlatego że chciałem się tylko na nim skupić.
Oczywiście, że w przypadku pisania z TDD czy ogólnie pisania z głową, taki kod nie powinien nigdy się pojawić. Post nie miał na celu także pokazywania jedynego/słusznego podejścia do obiektówki. Dla mnie to jest rzeczywistość zastana, chciałem się podzielić swoimi rozmyśleniami nad tym, jak się lepiej zabezpieczyć przed przed wszelkimi zmianami, a w raczej zmianami, które będą powodować zmianę logiki.
Użycie magic number tutaj, jest tylko dla obsługi kolejnego przypadku w switch, żadna głębsza logika się za tym nie kryje. Równie dobrze mogłem wybrać liczbę 42.
Mam nadzieję, że trochę się wytłumaczyłem.
Przykłady nie są przejrzyste. Zamiast pisać tyle tych metod testowych, mogłeś po prostu użyć TestCase-y. Przykład dla NUnita (widzę, że z niego korzystasz):
[TestCase(1, Result = 1)]
[TestCase(2, Result = 4)]
[TestCase(3, Result = 8)]
[TestCase(4, Result = 0)]
public int Method_Returns_Proper_Value_For_Input(int input)
{
SwtichFun target = new SwtichFun();
return target.Method(input);
}
Postaram się znaleźć chwilę i wrzucić czytelniejszą wersję testów.