Thanks Vladimir. Most of the changes look find to me (left a small comment in PR)
Once merged, I'll take care of remaining JUnit4 rules for Cassandra/ElasticSearch/Mongo etc. On Tue, Dec 3, 2019 at 1:23 PM Vladimir Sitnikov < sitnikov.vladi...@gmail.com> wrote: > Ok, I think PR1621 is ready to go: > https://github.com/apache/calcite/pull/1621 > I'm inclined to merge it shortly > > The PR converts most of the tests, and by default, it allows JUnit5 only. > > Certain modules still have "non-trivial" JUnit4 code, and there was an > agreement to convert them later. > The modules receive junit4 dependency via junit4=true in gradle.properties. > > --- > > The most "controversial" change from my point of view is the removal of > *Suite* classes. > In other words, I have dropped classes like > CalciteSuite, FileSuite, PlusSuite, and so on. > > I guess it is OK, as both Gradle and JUnit5 provide lots of ways to launch > tests (e.g. tags are good). > The good thing about the removal is it would reduce the number of > merge-conflicts, > and it would simplify adding new test classes. > > --- > > The most notable difference between junit4 and junit5 .assert* is that > junit4 has (message, expected, actual) > argument order while junit5 uses (expected, actual, message). > The latter is better when message is big (e.g. concatenated), and junit5 > allows to use lambda, > so the message is built only when assertion fails. > > I've performed argument shuffling with IntelliJ IDEA's "structural search > and replace", so it should be OK. > > Vladimir >
