You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by smoyer64 <gi...@git.apache.org> on 2018/09/05 01:06:50 UTC
[GitHub] commons-lang pull request #353: WIP: Update tests to JUnit5 via @boyarsky's ...
GitHub user smoyer64 opened a pull request:
https://github.com/apache/commons-lang/pull/353
WIP: Update tests to JUnit5 via @boyarsky's converter
This PR updates many of the commons-lang tests to JUnit5. I'm providing this PR simply to illustrate how much of the work can be done via the automated tool but this is clearly __NOT__ ready to merge. If this looks promising to the commons-lang maintainers, I'll work on fixing the portion of the tests that didn't convert properly. Better yet, if others are interested in helping with this effort and believe that the converter gives us a pretty good head start, I'd be happy to collaborate on the branch I've created.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/selesy/commons-lang master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-lang/pull/353.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #353
----
commit ee5b9289565649638f201c762f0857b67a8762aa
Author: Steve Moyer <sm...@...>
Date: 2018-09-05T01:00:40Z
Update tests to JUnit5 via @boyarsky's converter
----
---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Posted by britter <gi...@git.apache.org>.
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...
Posted by kinow <gi...@git.apache.org>.
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!
---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Posted by britter <gi...@git.apache.org>.
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 pull request #353: WIP: LANG-1416: Update tests to JUnit5 via @...
Posted by smoyer64 <gi...@git.apache.org>.
Github user smoyer64 closed the pull request at:
https://github.com/apache/commons-lang/pull/353
---
[GitHub] commons-lang issue #353: WIP: LANG-1416: Update tests to JUnit5 via @boyarsk...
Posted by smoyer64 <gi...@git.apache.org>.
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...
Posted by smoyer64 <gi...@git.apache.org>.
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...
Posted by smoyer64 <gi...@git.apache.org>.
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...
Posted by kinow <gi...@git.apache.org>.
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...
Posted by kinow <gi...@git.apache.org>.
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 :-)
---