-
Notifications
You must be signed in to change notification settings - Fork 127
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
#1246 add Home.load() function #1344
Conversation
@volodya-lombrozo please run the build locally with Qulice enabled within parent module:
|
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.
Code style fixes are required
@mximp pls, take a look for recent changes |
} | ||
|
||
@Test | ||
void loadFromAbsentFile(@TempDir final Path temp) { |
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.
@volodya-lombrozo it's better to use Assertions.assertThrows
from JUnit 5
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.
Done. BTW, can we use JUnit assertions instead of MatcherAssert
in orher places?
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.
Done. BTW, can we use JUnit assertions instead of
MatcherAssert
in orher places?
@volodya-lombrozo we use them somewhere, but JUnit assertions are way less powerful than Hamcrest.
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.
@volodya-lombrozo For consistency I'd use MatcherAssert
everywhere for matching.
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.
@mximp Even for exceptions?
@volodya-lombrozo maybe it's good to use this new function somewhere in the code? Otherwise, it's not clear what it's for... |
I've used |
@rultor merge |
} | ||
|
||
@Test | ||
void loadFromAbsentFile(@TempDir final Path temp) { |
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.
@volodya-lombrozo For consistency I'd use MatcherAssert
everywhere for matching.
Add
Home.load()
function together with unit tests. Also small refactoring was perfomed in the correlated files.Issue: #1246