-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
I have written all the tests #791
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
I guess there are a several mistakes. It would be great if you check your code once more)
void readFromFile_pathToFile_ok() { | ||
List<String> expectedLines = List.of( | ||
"b,banana,20", | ||
"b,apple,100", | ||
"s,banana,100", | ||
"p,banana,13", | ||
"r,apple,10", | ||
"p,apple,20", | ||
"p,banana,5", | ||
"s,banana,50" | ||
); | ||
List<String> lines = readerService.readFromFile(PATH_TO_FILE); | ||
assertEquals(expectedLines, lines); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's better to use words "expected"/"actual" intead of "expectedLines"/"lines", or at least "expectedLines"/"actualLines".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with @Alexsandr-Yablunovskyi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL, I renamed all variables beside except where it says
} | ||
|
||
@Test | ||
void parse_wrongAmount_notOk() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to call method "invalidAmount" because you have used word "Invalid" in previous and next methods.
List<String> lines = List.of("b,apple,100", "s,apple,20", | ||
"s,apple,30", "p,apple,ten"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I catch NumberFormatException in my ParseServiceImpl when I parse String to Integer
+ "banana,10" + System.lineSeparator() | ||
+ "apple,40"; | ||
writeService.writeToFile(PATH_TO_FILE, report); | ||
String actualReport = readFromFile(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if you test "writePathToFile" method you should not use another methods from your code (You shouldn't use readFromFile())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It`s for the KISS principle. I created a new method here outside of my other classes. Maybe, you are right, but I'll leave it like this for now.
private static Operation balanceOperation; | ||
|
||
@BeforeAll | ||
static void setUp() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that setUp() method uses with @beforeeach annotation. And for @BeforeAll it's better to use beforeAll()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good solution! Let's improve it)
void readFromFile_pathToFile_ok() { | ||
List<String> expectedLines = List.of( | ||
"b,banana,20", | ||
"b,apple,100", | ||
"s,banana,100", | ||
"p,banana,13", | ||
"r,apple,10", | ||
"p,apple,20", | ||
"p,banana,5", | ||
"s,banana,50" | ||
); | ||
List<String> lines = readerService.readFromFile(PATH_TO_FILE); | ||
assertEquals(expectedLines, lines); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with @Alexsandr-Yablunovskyi
} | ||
|
||
@Test | ||
void readFromFile_pathToFile_ok() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void readFromFile_pathToFile_ok() { | |
void readFromFile_validPathToFile_ok() { |
String expected = "fruit,quantity" + System.lineSeparator() | ||
+ "banana,10" + System.lineSeparator() | ||
+ "apple,40"; | ||
assertEquals(expected, reportCreatorService.createReport()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use StringBuilder
assertEquals(expected, actual); | ||
} | ||
|
||
private static String readFromFile() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why static?)
} | ||
|
||
@Test | ||
void writeToFile_writePathToFile_ok() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u can add test for path = null
} | ||
|
||
@Test | ||
public void performOperation_enoughFruits_notOk() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong name
No description provided.