Mike, Thanks for moving things forward on the JUnit 5 migration. I posted a couple comments on the PR for nifi-commons (NIFI-9080) with concerns related to breaking tests and perpetuating ignored unit tests.
Summarizing in this thread for general discussion, I am concerned about breaking unit tests as part of the migration process. As long as NiFi supports both JUnit 4 and JUnit 5, the migration should improve the overall testing environment. Breaking existing tests will require additional work to go back and fix, and spending the effort upgrading and reviewing test classes doesn't seem worth it if we continue marking tests as ignored. In some particular cases, it may be worthwhile to remove a test method or test class. For test methods related to performance, migrating to JUnit 5 seems like an ideal opportunity to make tests conditional on environment variables or system properties. I would be glad to help with the migration process, but it would be helpful to avoid having to revisit code multiple times to address these issues. Regards, David Handermann On Wed, Aug 25, 2021 at 2:24 PM Mike Thomsen <mikerthom...@gmail.com> wrote: > I broke up the tickets because it is A LOT of individual tasks that > can break the overall build and wanted to scope the work appropriately > so that people looking for a chance to contribute could snatch up a > few easy wins. > > I'm still going to take point on making the changes, but the plan > going forward is to submit PRs that bundle a bunch of tickets so that > we don't overwhelm reviewers and the CI/CD pipeline. > > Most of the changes are automated by IntelliJ Ultimate's migration > tool, which from my testing is really good at migrating about 95% of > our JUnit 4 unit and integration tests. > > Thanks, > > Mike > > On Wed, Aug 25, 2021 at 1:05 PM Joe Witt <joe.w...@gmail.com> wrote: > > > > Joey > > > > I personally dont care all that much about the number of commits in PR > > - I think that rule is sort of soft already. > > > > I dont think there is any inherent value in having a single module per > > JIRA (and PR or PR commit) on this. These can be done in much coarser > > grained chunks. It will have to be to get review cycles for instance > > (much less having the Github infra to run these builds). > > > > Thanks > > Joe > > > > On Wed, Aug 25, 2021 at 9:44 AM Joey Frazee > > <joey.fra...@icloud.com.invalid> wrote: > > > > > > Maybe this is an exception to the single squashed commit guidance for > the initial pull? > > > > > > I assume the intent is to make incremental progress and not have a PR > with a hundred files affected, but if the different module changes > corresponded to a different commit, GH will make it easy enough to have a > draft and review each commit in isolation. > > > > > > Would that be a reasonable approach? > > > > > > -joey > > > > > > > On Aug 25, 2021, at 9:36 AM, Joe Witt <joew...@apache.org> wrote: > > > > > > > > Mike, > > > > > > > > Seeing a pretty stunning flood of JIRAs for 'Refactor nifi-bla to use > > > > JUnit 5' and I'm guessing we'll see the same in terms of PRs. This > is > > > > a really high administrative overhead approach to this. > > > > > > > > Why not break this into one or maybe a few JIRAs/PRs total? > > > > > > > > Thanks >
