Hi Oleg, I can outline some motivating ideas. There is no agreed solutions yet, but junit4 might help us in reusing existing tests for improving MVCC coverage. Another area is managing RunAll execution time with help of some kind of tests parameterization.
Regarding concrete tests changes from my experiment [1]. They were changed in order to demonstrate certain capabilities in action. 1. CacheMvccDmlSimpleTest simply demonstrates that it is a valid junit4 test. Test names without "test" prefix make it evident that tests are run by junit4 engine. 2. IgniteCacheCommitDelayTxRecoveryTest shows how a pattern common for our junit3 tests could be improved by junit4. I did not have each test method renaming in mind (but I suppose we cannot avoid marking each test with @Test annotation). And I also have run RunAll after introducing changes in GridAbstractTest [2]. There were several rebuilds because of problems found along the way. I will return to it in order to receive good enough single RunAll result eligible for analysis by human. [1] https://github.com/apache/ignite/pull/5354/files [2] https://mtcga.gridgain.com/pr.html?serverId=apache&suiteId=IgniteTests24Java8_RunAll&branchForTc=pull/5354/head&action=Latest пт, 9 нояб. 2018 г. в 16:48, oignatenko <oignate...@gridgain.com>: > 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/ > -- Best regards, Ivan Pavlukhin
