milo
milo copied to clipboard
eclipse
Migrate from TestNG to JUnit5
- all compilation issues fixed
- remains few failing tests
Before you submit a pull request please acknowledge the following:
- [x] You have signed an Eclipse Contributor Agreement and are committing using the same email address
- [x] Your code contains any tests relevant to the problem you are solving
- [x] All new and existing tests passed
- [ ] Code follows the style guidelines and Checkstyle passes
See CONTRIBUTING for more information.
Please note that I'm on PTO next week. Feel free to update/modify if needed
Thanks for picking this one up. I'll try to take a closer look at this today or over the weekend.
On first glance it looks mostly good, except I think you missed the most annoying thing about the migration, which is that JUnit and TestNG assertions swap the position of the "expected" and "actual" parameters :crying_cat_face:
which is that JUnit and TestNG assertions swap the position of the "expected" and "actual" parameters
argh and there is 1335 matches just for assertEquals
and after a quick searchI have not found a reliable way to switch them automatically
Provided as a separate commit to more easily revert in case there is a problem for assertEquals using:
Careful, I think you might be hitting existing JUnit tests and swapping the positions of those assertions.
Careful, I think you might be hitting existing JUnit tests and swapping the positions of those assertions.
hum, I didn't noticed that there were existing Junit tests. Effectively I hit them. I need to stop for today and then I have a week of PTO. Feel free to pick the commits you are interested in, or to start from scratch.
IntelliJ allows you to specify the scope of a Find (and Replace) action, and it looks like local changes is one of those scopes. I might be able to get something working using that... I'll play with it a bit.
When talking about IntelliJ there is Structural Replace which allows you to find all method calls to the testng Assertion class and replace them with switched first and second parameters while leaving the remaining arguments unchanged (e.g. message) I would go with this, is has a steeper learning curve but is worth it.
@protogenes that's another good solution, though it would have to be applied as the first step, before all the package migration from TestNG to JUnit happens.
Would it be worth it to have a look at AssertJ in the context of the migration towards JUnit5? The fluent syntax helps with making the actual assertions a bit clearer to read and understand if there are multiple conditions being checked on a given result, e.g.:
assertEquals(5, nodes.size());
assertTrue(nodes.stream().anyMatch(n -> n.getNodeId().equals(Identifiers.Server_ServerArray)));
assertTrue(nodes.stream().anyMatch(n -> n.getNodeId().equals(Identifiers.Server_NamespaceArray)));
assertTrue(nodes.stream().anyMatch(n -> n.getNodeId().equals(Identifiers.Server_ServiceLevel)));
assertTrue(nodes.stream().anyMatch(n -> n.getNodeId().equals(Identifiers.Server_Auditing)));
assertTrue(nodes.stream().anyMatch(n -> n.getNodeId().equals(Identifiers.Server_EstimatedReturnTime)));
would become
assertThat(nodes)
.extracting(UaNode::getNodeId)
.containsExactlyInAnyOrder(
Identifiers.Server_ServerArray,
Identifiers.Server_NamespaceArray,
Identifiers.Server_ServiceLevel,
Identifiers.Server_Auditing,
Identifiers.Server_EstimatedReturnTime);
If this is something that might be of interest, I could start working on a PR to preview how this would look like if applied across the board.
@VSSW-GTN thanks but I'm not interested in adding another library. I've never been convinced that assertion "DSLs" like this end up being worth it in the end. Regular assertions aren't difficult to read and don't come with the overhead of needing to understand the assertion language of whatever library you're using. Maybe if the tests were complex enough it would start to make sense.