Refactor some utilities and TODO done #301
Conversation
Current coverage is
|
| @@ -170,7 +170,7 @@ public static boolean runInMultipleThreads(int numberOfThreads) throws Exception | |||
| boolean failed = false; | |||
| for (AllTestsRunner t : threads) { | |||
| t.join(); | |||
| failed = failed ? true : t.isFailed(); | |||
| failed = failed || t.isFailed(); | |||
There was a problem hiding this comment.
Yes! you're right i already made the changes thanks for your comments :)
Ariel-Isaacm
force-pushed
the
master
branch
3 times, most recently
from
10e41ef
to
cbbe83c
Compare
|
Commits squashed* |
|
Good job! The refactor is well done and the code is cleaner now. Maybe the coverage is gonna be decreased thanks to the null check. Maybe a new test will do. |
| if(start == null || separator == null || end == null || values == null){ | ||
| return ""; | ||
| //TODO handle this situation | ||
| } |
There was a problem hiding this comment.
I think you can just replace null by some default values of each fields
|
Looks good to me, I think you can just fixup the todo with my comment |
|
Hi, i did the changes, are they what you expected? and should i squash the two commits? |
|
Yes please. Thank you! |
Ariel-Isaacm
force-pushed
the
master
branch
2 times, most recently
from
30a8539
to
5550eb7
Compare
Refactor some utilities and TODO done Refactor some utilities and TODO done refactor for boolean condition and null check improved Refactor some utilities and TODO done Refactor some utilities and TODO done Refactor some utilities and TODO done refactor for boolean condition and null check improved Null check improved Default value changed
|
Done :) |
Refactor some utilities and TODO done
|
Thanks @Ariel-Isaacm for the contribution |
|
No, thank you! |
|
Yes exactly it was quite the opposite, thanks for the patience ;) |
Hey guys, awesome framework +1 i've been looking at the code and i did some improvements, also i did a TODO but i'm not quite sure how to handle that null check it would be awesome if you could check it out @bric3 @szczepiq
Answers.java : private is not required
ClassPathLoader.java: Removed unnecessary imports
ScenarioPrinter.java: Changed the string concat into another append
MockHandlerFactory.java: Return statement changed to be inline
ValuePrinter.java: TODO done, null check added
Edit
ReleaseNotesExtension : Made authToken a final String
Edit 2:
Some improvements to the ThreadTests :)