Updated update-method module to JUnit 5 #1542
Conversation
|
@charlesfinley This looks like a duplicate of #1541 |
This PR includes the changes from 1541 and adds more. #1541 can be closed in favor of this PR if necessary. |
There was a problem hiding this comment.
Please try to provide a detailed documentation in the comment section of this Pull Request of all the changes you have made to that It becomes easier for other reviewers.
Overall a Good Pull Request.
Thanks.
Wait for @iluwatar to make the final commit.
trampoline/src/main/java/com/iluwatar/trampoline/Trampoline.java
Outdated
Show resolved
Hide resolved
|
@charlesfinley thanks for the pull request! The JUnit 5 related changes look ok, but see my comment about the change to Trampoline pattern. |
ohbus
added
the
status: stale
Bringing my fork current with master
JUnit 4 is not needed when using JUnit-Vintage
ohbus
removed
the
status: stale
There was a problem hiding this comment.
LGTM
Thank you so much @charlesfinley for this contribution.
Can please you look at the static issues reported by sonarbot and kindly make the updates to fix it.
I can't seem to find the sonarbot issues - can you link me? I'm seeing all 3 checks passing. |
|
The 28 code smells that seems to be reported by the sonarbot comment. Here they are https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&pullRequest=1542&resolved=false&types=CODE_SMELL |
Looks like all of these pre-date my changes. I'll clean them up and push up a commit shortly. Edit - looks like the Tests updated to use JUnit 5 no longer require access modifiers. :) |
|
@ohbus Code smells resolved. |
There was a problem hiding this comment.
These are the final set of changes that would make this PR great!
Please look into this and we can merge this after!
Thank you so much for your changes @charlesfinley
| @Test | ||
| public void shouldExecuteWithoutException() { | ||
| void shouldExecuteWithoutException() { | ||
| assertDoesNotThrow(() -> SagaApplication.main(new String[]{})); | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add an extra line here
There was a problem hiding this comment.
You want an empty line after the method declaration and before the assertion?
There was a problem hiding this comment.
Yes an empty line at the end of the file.
There was a problem hiding this comment.
Ah, duh. I need coffee. Updated!
|
|
||
| @Test | ||
| public void mainTest() { | ||
| void mainTest() { | ||
| SagaApplication.main(new String[]{}); | ||
| } | ||
| } |
update-method/src/test/java/com/iluwatar/updatemethod/WorldTest.java
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
ohbus
dismissed
iluwatar’s stale review
The changes requested has been resolved.
Updated update-method module to JUnit 5