Hi guys, I started writing a junit4 test for a feature I am currently working on. And immediately I faced a peculiarity with @Before test fixture. I automatically named a method "setUp" and realized that I override a method from a superclass. It can produce unwanted effects. First, it looks reasonable to call a corresponding superclass method in an overridden method, but in my understanding it is not a canonical way in junit4. Second, in junit4 overriding method annotated with @Before can lead changing order of this method execution among other @Before method. Second point might lead to surprising effects in tests hierarchies defining @Before methods on different levels.
Let's return to the test I mentioned in the beginning of current email. I decided to use a different name for my @Before method (actually it was unpleasant "setUp1"). I think that we can do the following to make things straightforward: 1. Prohibit overriding base class setUp and tearDown methods by making them final (at least in GridCommonAbstractTest and GridSpiAbstractTest). 2. Use common default name for @Before and @After methods defined in concrete test classes. I do not have really good candidates for names. Simply we can use setUp1, tearDown1. Another option initTest, cleanupTest. Your name candidates are quite appreciated. WDYT? P.S. Also I went through other tests and found really dangerous thing. In IgniteJmsStreamerTest (not migrated to junit4 yet) we have beforeTest method (overriding GridAbstractTest framework method) annotated with @Before annotation (I believe accidentally)! It means that migrating such test to junit4 will lead to calling beforeTest twice. Ouch! ср, 12 дек. 2018 г. в 13:21, Павлухин Иван <vololo...@gmail.com>: > > Hi Oleg, > > Thanks your for detailed answer. Using junit4 annotations where > possible sounds good to me. > > Also I would like to thank everybody involved in migration to junit4 > process for your efforts. You were able to move a problem which looked > mountain-heavy. I believe that is a significant milestone on Ignite > stabilization road. > > Well done! > вт, 11 дек. 2018 г. в 20:20, oignatenko <oignate...@gridgain.com>: > > > > Hi Ivan, > > > > That's a very good question and I think you spotted a point where Ignite > > test developer's preferences need to change when migrating from Junit 3 to > > 4. > > > > In brief, when developing under JUnit 3 there were good reasons to prefer > > our homebrew template methods (beforeTestsStarted, beforeTest, afterTest, > > afterTestsStopped) over those provided by JUnit (setUp / tearDown). To > > answer your other question, in order to reliably understand how our methods > > work one would better go to (potentially changing) code of GridAbstractTest > > and check where and how these are invoked. > > > > But under JUnit 4 these reasons don't apply anymore and preference should go > > the other way, namely in favor of annotations @Before, @After, @BeforeClass, > > @AfterClass. > > > > In other words, when these annotations allow you implement desired test > > logic you better use them. These annotations have stable, well documented > > and widely understood semantics (and don't require one to skim through 3K > > lines of code for grasping details:). > > > > And it makes sense to "fallback" to our old-fashioned methods only when you > > simply have no other choice. For example doing some quick and simple fix in > > a legacy code that is filled with multiple usages of our old template > > methods it may be okay to choose to keep old style - instead of doing > > cumbersome, risky and lengthy refactoring or polluting test code with > > inconsistent use of annotations intertwined with old style methods. > > > > That's what I think. If you're interested in why I think so, refer to boring > > explanation in PS. > > > > regards, Oleg > > > > ----- > > > > PS. With regards to old preferences I believe it is very important to keep > > in mind that Junit 3 didn't provide means to do initialization and cleanup > > once for all test methods in the class. > > > > So when one needed things to run once per test class they had to go > > somewhere else. In Ignite the natural way to go was GridAbstractTest which > > provided this functionality via beforeTestsStarted and afterTestsStopped > > (with some glitches as I recently learned but still). And after one had to > > use these methods there was no compelling reason to use JUnit's setUp and > > tearDown, because GridAbstractTest also provided similar methods. > > > > It was just easier to consistently use full and cohesive set of methods > > provided by GridAbstractTest than switch back and forth between it and JUnit > > 3 methods. > > > > With JUnit 4 above reasoning doesn't apply anymore because its features are > > just as full and cohesive and can be consistently used without having to > > fallback to stuff from GridAbstractTest. So we have a real choice now and > > can judge based on other criteria than functional completeness (since both > > ways now satisfy that most important criteria). > > > > And this reveals us a full spectrum of JUnit advantages over > > GridAbstractTest (which was obscured before, because JUnit 3 was > > functionally incomplete). > > > > And on that newly opened spectrum JUnit 4 API wins hands down. > > > > JUnit 4 is much better documented and "googlable", it is stable and much > > wider known. The last but not the least, as I already mentioned, it doesn't > > require one to skim through 3K lines of code for grasping details and its > > semantics doesn't depend on (possibly changing) implementation code. > > > > Also, initialization and cleanup methods of JUnit 4 make an organic part of > > wider, more comprehensive API which smoothly integrates with IDE, Maven, > > Teamcity and all imaginable Java tools over there. > > > > Every time one annotates @Test or @Ignore gives them a reason to prefer > > other JUnit annotations over something else. It works about the same as I > > described above for beforeTestsStarted in GridAbstractTest and how it > > inhibited use of JUnit 3 methods: after you picked one part of some API it > > naturally becomes more convenient to keep using other parts of that API. > > > > Summing up, I believe that every time you have a (sensible) choice between > > JUnit 4 and GridAbstractTest methods you just pick JUnit 4 and don't look > > back. > > > > > > Павлухин Иван wrote > > > Ed, Oleg, > > > > > > Could you please clarify on the following point: > > >> 3. Add @Before, @After on methods which should run before, after a > > >> test (setUp, tearDown in current approach). > > > Beforehand we used to override beforeTestsStarted, beforeTest, > > > afterTest, afterTestsStarted. What will happen with these methods > > > after migration to junit4? I can see that order of execution can > > > become tricky especially when both styles are used at the same time. > > > And an immediate suggestion is to avoid using both styles (annotations > > > and overridden methods) in the same test. What is your insight? > > > вт, 11 дек. 2018 г. в 10:27, Vladimir Ozerov < > > > > > vozerov@ > > > > > >: > > >> > > >> Hi Oleg, > > >> > > >> Yes, makes perfect sense. Thank you. > > >> > > >> On Mon, Dec 10, 2018 at 10:14 PM oignatenko < > > > > > oignatenko@ > > > > > > wrote: > > >> > > >> > Hi Vovan, > > >> > > > >> > I just created JIRA ticket to address your concerns: > > >> > - https://issues.apache.org/jira/browse/IGNITE-10629 > > >> > > > >> > In brief, the plan is that a week or two after migration is over we > > >> will > > >> > run > > >> > code inspection that detects JUnit 3 style tests that lack @Test > > >> annotation > > >> > and fix these tests if there are any. > > >> > > > >> > Does that answer your question? > > >> > > > >> > regards, Oleg > > >> > Vladimir Ozerov wrote > > >> > > Ed, > > >> > > > > >> > > Several questions from my side: > > >> > > 1) When the change is expected to be merged? > > >> > > 2) What contributors with opened PRs and new or updated JUnit3 tests > > >> are > > >> > > supposed to do? Rewrite to JUnit4? > > >> > > > > >> > > If yes, then we should give them time to have a chance to get used to > > >> new > > >> > > approach and resolve possible conflicts. > > >> > > > > >> > > Vladimir. > > >> > > > > >> > > пн, 10 дек. 2018 г. в 20:32, Eduard Shangareev < > > >> > > > >> > > eduard.shangareev@ > > >> > > > >> > > >: > > >> > > > > >> > >> Ivan, > > >> > >> > > >> > >> So, suggested actions with the new approach: > > >> > >> 1. Add @Test annotation on test methods. > > >> > >> 2. Add @RunWith(JUnit4.class) annotation on test class. > > >> > >> 3. Add @Before, @After on methods which should run before, after a > > >> > >> test (setUp, tearDown in current approach). > > >> > >> 4. Add your test-class to a suite using suite.addTest(new > > >> > >> JUnit4TestAdapter(YourTestClass.class)); > > >> > >> 5. Use @Ignore instead fail(); for muting test. > > >> > >> 6. You could start using @Parametrized instead of inheritance. > > >> > >> > > >> > >> > > >> > >> On Mon, Dec 3, 2018 at 1:15 PM Павлухин Иван < > > >> > > > >> > > vololo100@ > > >> > > > >> > > > wrote: > > >> > >> > > >> > >> > Hi Oleg, > > >> > >> > > > >> > >> > I noticed that GridAbstractTest is now capable to run junit4 > > >> tests. > > >> > >> > What are the current recommendations for writing new tests? Can we > > >> use > > >> > >> > junit4 annotation for new tests? > > >> > >> > пн, 12 нояб. 2018 г. в 19:58, oignatenko < > > >> > > > >> > > oignatenko@ > > >> > > > >> > > >: > > >> > >> > > > > >> > >> > > Hi Ivan, > > >> > >> > > > > >> > >> > > I am currently testing approach you used in pull/5354 in the > > >> "pilot" > > >> > >> > > sub-task with examples tests (IGNITE-10174). > > >> > >> > > > > >> > >> > > So far it looks more and more like the way to go. The most > > >> promising > > >> > >> > thing I > > >> > >> > > observed is that after I changed classes in our test framework > > >> the > > >> > >> way > > >> > >> > you > > >> > >> > > did, execution of (unchanged) examples tests went exactly the > > >> same > > >> > as > > >> > >> it > > >> > >> > was > > >> > >> > > before changes. > > >> > >> > > > > >> > >> > > This indicates that existing tests won't be affected making it > > >> > indeed > > >> > >> low > > >> > >> > > risk. > > >> > >> > > > > >> > >> > > After that I converted examples tests to Junit 4 by adding > > >> @RunWith > > >> > >> and > > >> > >> > > @Test annotations and tried a few, and these looked okay. > > >> > >> > > > > >> > >> > > Currently I am running full examples test suite and after it is > > >> over > > >> > >> I > > >> > >> > will > > >> > >> > > compare results to the reference list I made by running it prior > > >> to > > >> > >> > > migration. > > >> > >> > > > > >> > >> > > regards, Oleg > > >> > >> > > > > >> > >> > > > > >> > >> > > > > >> > >> > > -- > > >> > >> > > Sent from: > > >> http://apache-ignite-developers.2346864.n4.nabble.com/ > > >> > >> > > > >> > >> > > > >> > >> > > > >> > >> > -- > > >> > >> > Best regards, > > >> > >> > Ivan Pavlukhin > > >> > >> > > > >> > >> > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > -- > > >> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > >> > > > > > > > > > > > > > -- > > > Best regards, > > > Ivan Pavlukhin > > > > > > > > > > > > -- > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/ > > > > -- > Best regards, > Ivan Pavlukhin -- Best regards, Ivan Pavlukhin
