[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user smoyer64 commented on the issue: https://github.com/apache/commons-lang/pull/353 @kinow > Alternatively, you can update the PR with some base work that you think might be useful for the next iterations. Up to you If the intention is to use the conversion as an opportunity to review all the test classes, then completing the automatic conversion is probably a bad idea. Once the tests are running again, nobody's going to look at them right? Most of the changes made by the converter were boilerplate ... greater than 80% of the test classes only had changes to their imports. I could create a branch that was nothing more than the conversion if you're interested and people could pick off a class, optimize/update it and submit a single class as a PR. The conversion tool needs a little work but it's also capable of converting a single file at a time. Or I can drop the whole idea of automation. ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/353 I think so @smoyer64 , but not meaning giving up on the task, just split it into smaller changes, or a complete change but with less distractions from the main work. Alternatively, you can update the PR with some base work that you think might be useful for the next iterations. Up to you :-) ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user smoyer64 commented on the issue: https://github.com/apache/commons-lang/pull/353 Sounds like the vote is to close this PR then? I certainly don''t disagree that it's hard (impossible?) to review the conversion of the entire test "set". ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/353 >In general it's hard to review gigantic change sets which have been created automatically, so I'd welcome an approach where we migrate one test case after another. +1, and bonus points if we refactor some of the old test code, or maybe even cover extra parts of the code :-) :tada ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user britter commented on the issue: https://github.com/apache/commons-lang/pull/353 In general it's hard to review gigantic change sets which have been created automatically, so I'd welcome an approach where we migrate one test case after another. ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user britter commented on the issue: https://github.com/apache/commons-lang/pull/353 It looks like an approach that can potentially some some work. I personally would do it by hand. There is a lot of stuff that can't be detected automatically. For example we have some tests that run for loops. Those could be converted to be parameterized. ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user smoyer64 commented on the issue: https://github.com/apache/commons-lang/pull/353 @kinow I've found several areas where I think the tool can be improved but as an example of what a head start it provides I've spent a few minutes getting all the .../builder/... tests running. The ``@Test`` annotation for the failing tests all contain an ``expect`` attribute and were fixed by wrapping the test body in an ``assertThrows`` statement. Some of the formatting changes are really unnecessary ... it seems that the tool analyzes all classes (not just those match the conventional naming patterns for tests. It also rewrites classes that have no ``@Test`` annotations at all. Jeanne presented this tool at JavaOne last year and she knew that it could be improved. Perhaps I can convince her to move it to the JUnit Pioneer project! ---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/353 @smoyer64 it looks interesting! Still have to update some projects at work as well, so keen to investigate it later. But, do you know if there's a way to disable some formatting changes? I noticed several files have diffs consisting mainly of blank lines being removed... without those changes I think it would be much easier to review it. Thanks! ---
