Hi, I have rerun RunAll for the aforementioned PR [1]. Results [2] look very similar to ones which we currently have for master.
[1] https://github.com/apache/ignite/pull/5354/files [2] https://ci.ignite.apache.org/viewLog.html?buildId=2282588&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll пт, 9 нояб. 2018 г. в 17:30, Павлухин Иван <vololo...@gmail.com>: > > 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 -- Best regards, Ivan Pavlukhin
