Hi Ivan, That's a very good question. I would say the primary motivation in the task we discuss is to make the change at the minimal cost and risk. Or, as stated in description of IGNITE-10173 "move... gradually, smoothly and safely".
With this in mind I would say that main expected benefit is laid out at the top of ticket description: > Currently unit tests in the project are mix of two versions Junit 3 and 4. > This makes it hard to develop and maintain. Making all tests use the same > version Junit is intended to address this problem. Above goal is probably modest but given that this change is expected to take minimal effort it looks good enough for me. On a more general note I don't expect strong technical benefits of this change, at least not immediately. This is based on my studies of prior discussions related to this matter and of the existing tests. I believe that if there were any strong and obvious technical benefits we would be long aware of these and we would migrate to newer Junit long time ago. Observing that migration hasn't happened - despite JUnit 3 being obsolete for over 10 years now, and despite many more fundamental changes that successfully made it to Ignite in past years - it looks natural to assume that there are just no technical reasons that would be strong enough to justify investing much effort in the migration. --- Now about your experiment at ignite/pull/5354, I briefly skimmed it and here are some initial impressions. First the bad news, some parts look indeed "insane" to me. I am talking specifically about renames in CacheMvccDmlSimpleTest and changes to test design in IgniteCacheCommitDelayTxRecoveryTest. I believe that these are wildly out of scope of the discussed change because I firmly want to keep it limited to absolute minimum of changes (ideally only annotations with involved imports). This is because there are just too many tests to (manually) change. My rough estimate is there about 7,000 (seven thousands!) test cases to change in core module alone; and I guess there are few thousands more in other parts of the project. Doing any extra changes to these tests makes things much more complicated, effort consuming and risky than was initially supposed. On a brighter side, changes made to GridAbstractTest look worth further studying. At this point I still prefer to keep this class unchanged (because this gives us a bulletproof guarantee against regressions in all prior tests) but this may change after we do "pilot" changes to examples tests (IGNITE-10174). These pilot changes will help us estimate with concrete code how much effort could be involved in maintaining "twin" classes in sync (you are absolutely right pointing that this is concerning). If it turns out too complicated to sync we would probably switch to something like you have done in pull/5354 regards, Oleg Павлухин Иван wrote > Hi Oleg, > > Migrating to Junit 4 sounds as great idea. I believe everybody > is quite surprised when finds Junit 3 in Ignite. > But for me personally it is good to understand what problem we > are trying to solve? What benefits will Junit 4 give us? What are > the most painful moments with current testing framework? > (I my mind I draw a proposal which contains sections "Motivation", > "Alternatives", etc.) > > Also I must say that I made some experiments related to Junit4 > migration. And from the first glance it seems that the most > important Ignite test framework classes contain a huge amount of > utility methods and a little amount of code related to a test lifecycle. > These classes are GridAbstractTest and GridCommonAbstractTest. > Both are quite thick, but as already said it is mostly utility code > which could be extracted. I would be great to come to very thin base > test classes (or even to baseclass-less tests at all) and include all > kind of utilities without inheritance. > > But massive refactoring perhaps is more for future. The main point > here is that Ignite test framework is not complex at all and could be > easily adopted to Junit 4. > > Regarding twin classes. How to keep them in sync? If the migration > process will be paused halfway it could make things complicated in > future. > > Additionally, I have a wild idea how to marry junit 3 and 4 without > introducing twins. The idea looks ugly from OOP perspective, but > here it is. We can simply annotate overridden Junit 3 setUp and > tearDown methods with @Before and @After annotations. And > use runTest overridden method for Junit 3 and TestRule for Junit 4 > in order to run test methods in newly started thread. > > For all details you can see an experiment on github [1]. Among them > are ticks with @RunWith annotation making possible to run a test > inherited from TestCase as Junit 4 test. Still, I might have missed > something. > > Does it sound sane anyhow? > > [1] https://github.com/apache/ignite/pull/5354/files > > > ср, 7 нояб. 2018 г. в 19:57, oignatenko < > oignatenko@ > >: > >> Hello, >> >> I created JIRA task for this move, with detailed explanation and >> step-by-step subtasks, your comments are welcome: >> >> - https://issues.apache.org/jira/browse/IGNITE-10173 >> >> In brief, the plan is to gradually migrate the part of tests that still >> uses >> Junit 3 (hundreds if not thousands of those that depend on >> GridAbstractTest >> and its subclasses) to newer version. The trick proposed to avoid doing >> all >> this in one (big and risky) step is to introduce a Junit4-based "twin" of >> GridAbstractTest (and respective twins of its subclasses) and use it to >> gradually change tests to use that newer API instead. >> >> After above is over, Junit 3 and all its related stuff (including >> GridAbstractTest) could be safely removed from project. >> >> 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/
