Hi Oleg, I have not quite understood who is going to call (from standpoint of test framework) beforeTestsStarted, beforeTest, afterTest, afterTestsStarted methods? вт, 18 дек. 2018 г. в 23:31, oignatenko <oignate...@gridgain.com>: > > Hi Ivan, > > To answer your last question, yes, all the tests are to be marked with @Test > annotations, and those that are meant to be ignored are going to be marked > with @Ignore annotations. There is no exceptions to that, and these > annotations will work just as well on tests using our home brewed > beforeTestsStarted, beforeTest, afterTest, afterTestsStarted. > > For the sake of completeness, developers will also be able to use Before* / > After* annotations on their own methods in tests. The only exception > (clarified in respective javadocs) is that these annotations shouldn't be > used on overrides of our home brewed methods - and these methods, in > addition, will be recommended (not mandated) to avoid wia deprecation > notices. > > ----- > > As for accessing tests which have problems running under junit4, the way how > I search for these in current master is regex search in IDEA for > "addTestSuite.*class", that is I search in testsuites for entries that are > added using method "addTestSuite" with parameter class. > > Probably worth noting that some of the problems that were previously > blocking addition of particular tests have been resolved in the course of > working on IGNITE-10177 (https://github.com/apache/ignite/pull/5615). One > riddle that currently looks particularly difficult to crack is Teamcity > failures in "Queries 1", I even haven't yet figured how to reproduce these > locally. > > regards, Oleg > > > Павлухин Иван wrote > > Hi Oleg, > > > > Now concerns about junit3 elimination are clear for me. And I agree > > that it is worth to do it. By the way is it possible to access tests > > which have problems running under junit4? I would like to take a look. > > > > Also a clarifying bit regarding next migration steps is needed. Sorry > > if it was described but I missed it. Currently we have tons of tests > > which rely on our home brewed beforeTestsStarted, beforeTest, > > afterTest, afterTestsStarted. Are you going to mark them all with > > corresponding junit4 annotations? > > пн, 17 дек. 2018 г. в 19:13, oignatenko < > > > oignatenko@ > > > >: > >> > >> Hi Ivan, > >> > >> Per my cursory check of the code currently in master, we have 239 test > >> classes that couldn't migrate (that's probably about 500 or something > >> test > >> cases). For comparison, over 1600 classes migrated without problems. This > >> is > >> to answer your questions about how many tests are affected by change and > >> how many tests were not migrated yet. > >> > >> ----- > >> > >> I think we can say that only small percent of tests turned out sensitive > >> to > >> JUnit framework change. > >> > >> In the same time, due to very large overall amount of our tests we have > >> quite a big number as an absolute value. I point this because if amount > >> of > >> troublesome tests was smaller (say, order of magnitude smaller) I would > >> consider delaying migration until all tests reworked to blend totally > >> seamlessly into new version JUnit. This is doable - as sort of "pilot > >> exercise" me and Ed adjusted one test case this way - but the sheer > >> amount > >> of work on 200+ classes raises a question, whether it is worth it (I > >> think > >> it isn't). > >> > >> With regards to question 2, changes to TestCounters are farly trivial > >> (and > >> necessary) if you look at the code diff. Prior code counted amount of > >> test > >> cases in the class by JUnit 3 convention: it picked public void methods > >> without parameters starting with "test". Changed code counts test cases > >> as > >> JUnit 4 does: by checking if method is annotated with @Test. Change is > >> necessary because it will allow test developers fully use JUnit 4 > >> features > >> by adding test cases that don't follow old naming requirement. I > >> experimented with it a little and as far as I could see the old > >> TestCounters > >> indeed break when there are methods following new convention, it > >> triggered > >> afterTestsStopped too early. > >> > >> The answer to your question 3 lies in javadocs I added to runSerializer > >> declaration, or, more precisely, in TestCounters javadoc referred from > >> there. As you can see, current plan is to get rid of TestCounters fairly > >> soon (per IGNITE-10179) and this will also get rid of runSerializer so > >> this > >> is merely a temporary band aid to be removed soon. > >> > >> For the sake of completeness, my initial plan was to thoroughly > >> investigate > >> and test whether change from JUnit 3 to JUnit 4 would keep accessing > >> counters safe or not and only introduce runSerializer if it really does > >> but > >> after realising that this whole thing is likely to go away in a few days > >> from now I decided that it isn't worth wasting time and just temporarily > >> made it the way that is waterproof guaranteed to be safe. > >> > >> ----- > >> > >> Now, to answer your question whether it is an option for us to keep part > >> of > >> tests under JUnit 3, my answer is most definitely no. > >> > >> Main reason is that having part of tests under JUnit 3 will deprive us > >> ability to consistently use Ignore annotation and force us fallback to > >> old > >> way to fail the tests we want to ignore. This would kind of trash the > >> whole > >> purpose of migration because we won't be able to simplify and improve > >> maintenance of ignored tests. > >> > >> Another important reason is that keeping JUnit 3 will much complicate our > >> test framework code. We will have to implement and maintain two versions > >> of > >> TestCounters (see answer to your question #2 above). We will also have to > >> keep the code that currently determines first/last test in the class and > >> possibly even complicate it to account for two versions of the framework > >> - > >> compare that to current plan to simply get rid of all that code per > >> IGNITE-10179. > >> > >> The last but not the least, this makes it much more complicated to later > >> migrate to JUnit 5. Although this is currently not in the nearest plans > >> (IGNITE-10180) we eventually will want to (especially taking into account > >> that migration from JUnit 4 is said to be easy). Having JUnit 3 tests > >> would > >> much complicate this because we have no idea if JUnit 5 could > >> interoperate > >> with such an old version (and I see no reason why we would want to waste > >> our > >> time and efforts investigating and testing this). > >> > >> Summing up, I believe it is very well worth it for us to get rid of JUnit > >> 3 > >> completely. > >> > >> ----- > >> > >> With regards to making LegacySupport enabled only on purpose, at this > >> point > >> I see no reason to enforce this in code because I expect that deprecation > >> notices will do that job. > >> > >> If a developer writing new test or reworking an old one follows the > >> deprecation recommendations they will use JUnit 4 way instead of > >> deprecated > >> methods and you can see from the code that in this case LegacySupport > >> will > >> just transparently pass-through the test code without introducing > >> anything > >> else beyond. (Note we can reconsider and rework this later in case if it > >> turns out that my expectation doesn't hold). > >> > >> Does that answer your questions? > >> > >> regards, Oleg > >> > >> > >> Павлухин Иван wrote > >> > Hi Oleg, > >> > > >> > The things become challenging. Truly I do not see any trivial solution > >> > so far. Could you please outline main problems which we are facing > >> > today? And how many tests are affected? Some clarifying questions: > >> > 1. I know that setup->test->teardown threading was changed for junit4 > >> > tests, but actually I thought that it might affect only small number > >> > of tests. Am I right here? > >> > 2. Also I saw that in your experiment [1] some changes related to > >> > TestCounters were made. What is wrong with them? > >> > 3. Why do we need wrap test execution into critical section > >> > (runSerializer lock)? I thought that we always run tests serially. > >> > > >> > I generally like an idea of having workaround falling back to old test > >> > execution flow. But for me the most desired trait of things like > >> > LegacySupport is being lightweight and enabled only on purpose. And > >> > from the first glance current prototype looks a little bit > >> > complicated. As an alternative we can keep junit3 for troublesome > >> > tests, can't we? > >> > > >> > Also is there any vision how many migration problems do we have so far > >> > and how many tests was not migrated yet? > >> > вс, 16 дек. 2018 г. в 17:39, oignatenko < > >> > >> > oignatenko@ > >> > >> > >: > >> >> > >> >> Hi Ivan, > >> >> > >> >> As promised in my prior mail, here is the branch where I experimented > >> to > >> >> address concerns you raised: > >> >> - > >> >> > >> https://github.com/gridgain/apache-ignite/tree/ignite-10177-experimental > >> >> > >> >> I tested it locally and on Teamcity and it worked as intended. > >> >> > >> >> I think I managed to exactly reproduce execution sequence of JUnit 3 > >> test > >> >> case so that tests designed expecting it will run exactly as it was > >> >> before. > >> >> > >> >> As for troublesome APIs I used deprecation to warn developers agains > >> >> using > >> >> these and recommend what they need to use instead. > >> >> > >> >> If you are interested in closer studying the changes, class > >> >> GridAbstractTest1 is probably best as a starting point. This class is > >> a > >> >> temporary copy of GridAbstractTest made to minimise amount of editing > >> in > >> >> dependent classes while I was experimenting; in real implementation > >> (per > >> >> IGNITE-10177) this code is expected to be in GridAbstractTest. > >> >> > >> >> Also, I used ML testsuite to debug the changes I made, because it > >> >> contains > >> >> convenient mix of usecases and runs fast. > >> >> > >> >> Your feedback is much appreciated. > >> >> > >> >> 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
