> If we go for the bulk modifications approach why not migrating completely to JUnit5?
Most of the tests can be migrated programmatically (string/replace) there are some which need to be done manually (eg. JUnit special rules like EmbeddedElasticsearchPolicy [1] and parameterized tests ). > I remember that we had a discussion around the subject before but I cannot find the thread at the moment. See [2] for relevant thread. There were several concerns (at the time) : 1. Noise in commit history 2. Require use of newer versions of build tools (maven, IDE etc.) 3. Support of parallel execution of tests (from maven / gradle). Now supported in 5.5 I'm happy to revive CALCITE-2457 if people are interested https://github.com/apache/calcite/blob/ab136b5f76a4cb951e847fcba6b414c5e80dbbe6/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java [1] https://lists.apache.org/thread.html/3c2e87359442eefbce67823926d3701e2541df1c798025be4a31e779@%3Cdev.calcite.apache.org%3E [2] On Mon, Dec 2, 2019 at 2:16 PM Stamatis Zampetakis <[email protected]> wrote: > If we go for the bulk modifications approach why not migrating completely > to JUnit5? > > I remember that we had a discussion around the subject before but I cannot > find the thread at the moment. > > > On Mon, Dec 2, 2019, 5:58 PM Andrei Sereda <[email protected]> wrote: > > > Do you think it makes sense to perform a bulk string/replace for JUnit 4 > -> > > 5 annotations ? At least for simple non-parameterized / no-rules tests. > > This way IDE will automatically select correct annotations. > > > > I had some scripts prepared as part of CALCITE-2457 [1] > > > > https://issues.apache.org/jira/browse/CALCITE-2457 [1] > > > > On Mon, Dec 2, 2019 at 8:13 AM Michael Mior <[email protected]> wrote: > > > > > Thanks for noting Stamatis! > > > > > > If a particular group of tests is migrating to JUnit 5, it also > > > probably makes sense to use org.junit.jupiter.api.Assertions instead > > > of org.junit.Assert. Although the latter is still supported, I think > > > it makes sense to use things in the jupiter namespace where possible. > > > > > > -- > > > Michael Mior > > > [email protected] > > > > > > > > > Le lun. 2 déc. 2019 à 07:53, Stamatis Zampetakis <[email protected]> a > > > écrit : > > > > > > > > Hello, > > > > > > > > At the moment, we can use both JUnit 4 and 5 annotations in our code > > > base. > > > > Lately, I spend some time on debugging a few test cases only to > realize > > > > that old and new APIs do not mix very well. If you encounter problems > > > with > > > > your annotations then consider checking your imports. The problem > might > > > lie > > > > on the fact that you are using at the same time annotations from > > > different > > > > versions. > > > > > > > > Consider for example the following test. > > > > > > > > import org.junit.Assert; > > > > import org.junit.Test; > > > > import org.junit.jupiter.api.Disabled; > > > > > > > > public class SimpleTest { > > > > > > > > @Test > > > > @Disabled > > > > public void test() { > > > > Assert.fail("Should not run because it is disabled"); > > > > } > > > > } > > > > > > > > Contrary to what people might expect the test above will run and will > > > fail > > > > with an assertion error. Changing the imports (see below) will make > the > > > > test behave as expected. > > > > > > > > package org.apache.calcite; > > > > > > > > import org.junit.Assert; > > > > import org.junit.jupiter.api.Test; > > > > import org.junit.jupiter.api.Disabled; > > > > > > > > public class SimpleTest { > > > > > > > > @Test > > > > @Disabled > > > > public void test() { > > > > Assert.fail("Should not run because it is disabled"); > > > > } > > > > } > > > > > > > > Best, > > > > Stamatis > > > > > >
