Hackster & Refactoring #11
Conversation
banadiga
left a comment
banadiga
left a comment
There was a problem hiding this comment.
...
[INFO] Hackster service ................................... FAILURE [ 6.763 s]
[INFO] Storage service .................................... SKIPPED
[INFO] Storage service client ............................. SKIPPED
[INFO] Realtor service .................................... SKIPPED
[INFO] API gateway service ................................ SKIPPED
[INFO] spring-cloud ....................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
|
|
||
| @PostConstruct | ||
| public void test () { | ||
| System.out.println(rate); |
| public void checkIfOldNumberIsNotHackster() { | ||
| boolean isHacksterFalse = hacksterServiceClient.isHackster("321321312"); | ||
|
|
||
| for (int i = 0; i < 5; i++) { |
There was a problem hiding this comment.
Better use repository to persist Hackster with desired numberOfApartments first.
There was a problem hiding this comment.
This test tests exact situation when phone number become a hackster. No reason to prepare such data set, then no value from such test.
|
|
||
| boolean isHacksterTrue = hacksterServiceClient.isHackster("321321312"); | ||
|
|
||
| assertThat(isHacksterFalse, equalTo(false)); |
There was a problem hiding this comment.
Because it's clarify that number which wasn't hackster on beginning become a hackster after several calls
There was a problem hiding this comment.
perform assert as soon as you can
There was a problem hiding this comment.
Yeah, will remove first assertion as you were right and this behavior already covered.
| private MockMvc mockMvc; | ||
|
|
||
| @Test | ||
| public void checkIfNewPhoneNumberIsNotHakster() throws Exception { |
There was a problem hiding this comment.
I do not like naming
Use convention _
ex:
public void isHackster_newPhone() {}
There was a problem hiding this comment.
Not agree with Snake_Case, if you don't like this name we can discuss it, but change Java convention to something else doesn't make sense
| } | ||
|
|
||
| @Test | ||
| public void checkIfOldPhoneNumberIsNotHakster() throws Exception { |
There was a problem hiding this comment.
Will rename, but not use snake_case
| .andExpect(status().isOk()) | ||
| .andExpect(content().string("false")); | ||
|
|
||
| for (int i = 0; i < maxAllowedApartmentsPerRealtor - 1; i++) { |
There was a problem hiding this comment.
That phone which was not a hakster will become a hackster after configured number of calls
|
|
||
| @Test | ||
| public void checkIfOldPhoneNumberIsNotHakster() throws Exception { | ||
| mockMvc.perform(get("/hackster/321321")) |
There was a problem hiding this comment.
It is the same test with loop ?
There was a problem hiding this comment.
No, this is totally different cases using same functionality.
| mockMvc.perform(get("/hackster/321321")); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
please leave contextLoads in HacksterServiceApplicationTests and move other test to HacksterServiceIntegrationTests
hackster-service-client/pom.xml
Outdated
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
| <java.version>1.8</java.version> | ||
| <spring-cloud.version>Dalston.SR1</spring-cloud.version> |
There was a problem hiding this comment.
use <spring-cloud.version>Dalston.RELEASE</spring-cloud.version>
|
|
||
| boolean isHacksterTrue = hacksterServiceClient.isHackster("321321312"); | ||
|
|
||
| assertThat(isHacksterFalse, equalTo(false)); |
There was a problem hiding this comment.
perform assert as soon as you can
3 participants
NOTICE : As Config Server gets configuration from master branch then current branch is can't build. Please, review then merge and only then try to play with it.