Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

found some mistakes #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

found some mistakes #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/test/java/sk/stuba/fei/uim/vsa/pr1/StudentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void ST04_shouldUpdateStudent() {
student = thesisService.updateStudent(student);
assertNotNull(student);
assertEquals(randomSetStringForTesting, getFieldValue(student, stringField, String.class));
student = thesisService.getStudent(Student01.aisId);
student = thesisService.getStudent(Student01.id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Viem ale nevedel som ako to opraviť tak som to aspoň ilustroval aby ste vedeli o chybe.

assertNotNull(student);
assertEquals(randomSetStringForTesting, getFieldValue(student, stringField, String.class));
assertNotEquals(originalStringValue, getFieldValue(student, stringField, String.class));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/sk/stuba/fei/uim/vsa/pr1/TeacherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void TE04_shouldUpdateTeacher() {
teacher = thesisService.updateTeacher(teacher);
assertNotNull(teacher);
assertEquals(randomSetStringForTesting, getFieldValue(teacher, stringField, String.class));
teacher = thesisService.getTeacher(Teacher01.aisId);
teacher = thesisService.getTeacher(Teacher01.id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rovnaké ako v komentári vyššie pri Student01.

assertNotNull(teacher);
assertEquals(randomSetStringForTesting, getFieldValue(teacher, stringField, String.class));
assertNotEquals(originalStringValue, getFieldValue(teacher, stringField, String.class));
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/sk/stuba/fei/uim/vsa/pr1/ThesisTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void TH02_1_shouldAssignThesisToStudentWithAlreadyAssignedThesis() {
testToHaveAnIdField(thesis02, thesisIdField);
Long thesis02Id = getEntityId(thesis02, thesisIdField);

thesis02 = thesisService.assignThesis(thesis02Id, studentId);
thesis02 = thesisService.assignThesis(thesis02Id, studentId); // toto som zakazal, prislo mi to logicke
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je to možné že sa to riešilo musím však nájsť kde presne.

assertNotNull(thesis02);
thesis02 = thesisService.getThesis(thesis02Id);
assertNotNull(thesis02);
Expand Down Expand Up @@ -508,7 +508,7 @@ void TH09_shouldUpdateThesis() {
String teacherField = teacherFields.get(0);
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja som to tak žiaľ bral, tak teda som to mal asi tušiť ospravedlňujem sa za zbytočný pull request.

thesis = thesisService.updateThesis(thesis);
assertNotNull(thesis);
thesis = thesisService.getThesis(thesisId);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/sk/stuba/fei/uim/vsa/pr1/bonus/PageableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void shouldGetPageOfStudents() throws NoSuchFieldException, IllegalAccessExcepti
students = pagedThesisService.findStudents(Optional.empty(), Optional.of("3"), pageable);
assertNotNull(students);
assertNotNull(students.getContent());
assertEquals(5, students.getContent().size());
assertEquals(0, students.getContent().size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

assertEquals(5, students.getTotalElements());
assertEquals(1, students.getTotalPages());
assertEquals(0, students.getPageable().getPageNumber());
Expand Down