Cha0z / springBootHomeWork

0 stars 1 forks source link

Code review #8 Log&Test #8

Open hjvfyfyfy opened 5 years ago

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/test/java/com/homework/home/MessageFactoryTest.java#L11

клави тестів мають знаходитися в семетричеих пакетів, тобто MessageFactoryTest.java має бути в src/test/java/com/homework/home/pattern/factory/ а не в src/test/java/com/homework/home/

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/test/java/com/homework/home/MessageFactoryTest.java#L28

тут требулоб просто передати null а не MessageType.FOR_TEST, це достатьоньо дивно створювати новий енам для тестів

hjvfyfyfy commented 5 years ago

На випадок якщо ми створимо новикй тип енаму в забудемо його захендляти можна було б написати тест який б беребирав всі енами і двився що буде

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/test/java/com/homework/home/OwnMapTest.java#L24-L27

тести не покривають всі варіанти поведінки метод, в тебе метод має 3 варіанти, цей https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L45-L46 цей https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L49-L50 і цей https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L52-L54

тобто тобі треба перевітити що метод зробить якщо дубліката немає, якщо він є, і тд

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/test/java/com/homework/home/OwnMapTest.java#L37-L43

Random в тестах погана ідея, якскільки те не можеш відтворити тест, не можеш взнати врикому значенні тест впав, якщо він впаде

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/test/java/com/homework/home/OwnMapTest.java#L30-L35

тей тест буде бавати еррори коли Random видасть як мінім два однакові значення. Задача на + яка ймовірність того що таке станиться ? І при яких умав цей тест буде завжди валитися?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L28-L30

якщо комнути ще трохи глибше то це не зовсім коректний тест, оскільки в unit tests ми тестуємо окремі unit, а метод getSize залежить від put. Тому яким чином ми можемо протемтувати по суті гетер

hjvfyfyfy commented 5 years ago

Також було б добре написати тести і для інших методів, як приватних так і для публічних

hjvfyfyfy commented 5 years ago

щодо Interceptor тут описано як додати в логи request Id JavaBestPracticesPresentation2.pptx

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/authorization/Interceptor.java#L1-L33 попробув витягнути request body i response body поста. ти не перевизначив afterCompletion вони виведе тільки реквест без респонса (здається про це я забув згадати)

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L42 який петер логів ?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/5d972aa28779bd2d19bcfc12407788280d9ff1a0/src/main/java/com/homework/home/map/OwnMap.java#L40-L62

з логі зрозуміло тільки одне що викликали метод, не зрозуміло ви там були козізіцї чи ні, як добавив ще якісь логи в метод що це прояснити