found some mistakes #3
Conversation
| assertNotNull(student); | ||
| assertEquals(randomSetStringForTesting, getFieldValue(student, stringField, String.class)); | ||
| student = thesisService.getStudent(Student01.aisId); | ||
| student = thesisService.getStudent(Student01.id); |
There was a problem hiding this comment.
Student01 je trieda v súbore TestData, ktorá obsahuje iba súbor testovacích hodnôt, nemá nič s tím ako je implementovaná entita a už vôbec nemá atribút id.
Spúšťal si si to s tvojimi zmenami?
There was a problem hiding this comment.
Viem ale nevedel som ako to opraviť tak som to aspoň ilustroval aby ste vedeli o chybe.
| assertNotNull(teacher); | ||
| assertEquals(randomSetStringForTesting, getFieldValue(teacher, stringField, String.class)); | ||
| teacher = thesisService.getTeacher(Teacher01.aisId); | ||
| teacher = thesisService.getTeacher(Teacher01.id); |
There was a problem hiding this comment.
Rovnaké ako v komentári vyššie pri Student01.
| assertNotNull(getFieldValue(thesis, teacherField, teacherClass)); | ||
| assertEquals(teacher01Id, getEntityId(getFieldValue(thesis, teacherField, teacherClass), teacherIdField)); | ||
| thesis = setFieldValue(thesis, teacherField, teacher02); | ||
| thesis = setFieldValue(thesis, teacherField, teacher02); // zakazal som kradnutie prace a teda zmenu ucitela |
There was a problem hiding this comment.
Nebral by som to ako kradnutie práce. Zmena vedúceho práce je bežná záležitosť, ktorá má svoje opodstatnenie. Nakoľko učiteľ je iba jeden z atribútov práce, ak zachováme, že môže byť v práci uvedený práve jeden je to absolútne v poriadku.
There was a problem hiding this comment.
Ja som to tak žiaľ bral, tak teda som to mal asi tušiť ospravedlňujem sa za zbytočný pull request.
| Long thesis02Id = getEntityId(thesis02, thesisIdField); | ||
|
|
||
| thesis02 = thesisService.assignThesis(thesis02Id, studentId); | ||
| thesis02 = thesisService.assignThesis(thesis02Id, studentId); // toto som zakazal, prislo mi to logicke |
There was a problem hiding this comment.
Je to možné že sa to riešilo musím však nájsť kde presne.
| assertNotNull(students); | ||
| assertNotNull(students.getContent()); | ||
| assertEquals(5, students.getContent().size()); | ||
| assertEquals(0, students.getContent().size()); |
There was a problem hiding this comment.
Toto nie je dobre. Z popisu PR je zrejmé, že metódu Pageable.next() máš zrejme zle implementovanú nakoľko tá metóda má vrátiť novú inštanciu Pageable nie upraviť pôvodnú. Takže posledné volanie findStudents() pracuje práve s Pageable(0, 5).
There was a problem hiding this comment.
Ah tak, tak to som zle vydedukoval, do budúcna by som možno urobil podrobnejší popis zadania. Viem, že to asi nie je jednoduché myslieť na všetko ale bolo tam veľa veci čo sme museli sami dedukovať/pýtať sa na dc a to je čas a práca navyše zase pre vás.
2 participants
Tieto testy mi neprechádzajú a domnievam sa že to tak nemá byť.
Commitnuty kod - pri každom teste, ktorý mi nezbehol som buď zmenil(okomentoval) riadky prislúchajúce chybám alebo opravil ak sa to dalo bez väčších zmien.
- vyriešil by som to hľadaním študenta podľa id(ilustračná zmena v kóde, viem že to nezbehne)
- vyriešil by som to hľadaním Teachera podľa id(ilustračná zmena v kóde)
- keďže je v pageable nastavená stránka 1 na vstupe musíme zmeniť aj posledný assert z 0 na 1(zabudol som to dať do commitu spolu) => assertEquals(1, students.getPageable().getPageNumber());
Ak sa mýlim, v niečom a bolo toto zbytočné tak sa vopred ospravedlňujem