Hello! Thank you Dmitriy, and thanks to everybody who participated in discussions.
I have created tickets for next steps: https://issues.apache.org/jira/browse/IGNITE-9210 (with subtasks) https://issues.apache.org/jira/browse/IGNITE-9222 https://issues.apache.org/jira/browse/IGNITE-9223 As usual, feedback will be very welcome. Regards, -- Ilya Kasnacheev 2018-08-07 13:58 GMT+03:00 Dmitriy Pavlov <dpavlov....@gmail.com>: > Hi Igniters, > > I've merged chages for following tickets > IGNITE-7615: Find orphaned tests without test suites, create separate test > suite for them; > IGNITE-8344: Remove duplicate tests and suites; > IGNITE-8345: Streamline tests' class names: mark Abstract and Load tests > obviously so; > > After including these suites we have now more than 100 occurrences of > //suite.addTest > > These tests were created early but not executed on TeamCity. If you are > interseted in test coverage increase and can contribute each of these suite > actualization, please feel free to create ticket for such suites > resurrection (or group of suites). > > Ilya, thank you for contribution and for your efforts to make this happen. > > Sincerely, > Dmitriy Pavlov > > ср, 1 авг. 2018 г. в 12:52, Dmitriy Pavlov <dpavlov....@gmail.com>: > > > Hi Ilya, > > > > could you please actualize this PR. TC Bot can now detect newly > > contributed tests' failures, so I think it is best point to apply you > > change. > > > > Sincerely, > > Dmitriy Pavlov > > > > пт, 25 мая 2018 г. в 18:16, Eduard Shangareev < > eduard.shangar...@gmail.com > > >: > > > >> Igniters, > >> > >> While making review I checked next main-method tests: > >> > >> org.apache.ignite.loadtests.mapper.GridContinuousMapperLoadTest1 > >> org.apache.ignite.loadtests.mapper.GridContinuousMapperLoadTest2 > >> > >> And I have found that they are totally outdated! > >> They use config which was changed a long time ago. > >> And use localPeek with parameters which don't make sense now. > >> > >> So, I suggest to delete them. > >> > >> If there wouldn't be any objection I will do it myself. > >> > >> > >> > >> > >> On Tue, May 22, 2018 at 6:59 PM, Ilya Kasnacheev < > >> ilya.kasnach...@gmail.com> > >> wrote: > >> > >> > Hello, Igniters! > >> > > >> > One moment more of your time. One, we seem to have a consensus now > that > >> > tests should be added to suites, but commented out. They should be > >> > uncommented out later, for which numerous tickets will be created. > This > >> way > >> > we can tackle. > >> > > >> > Another issue sprang up, just now I have discovered an 'ignored-tests' > >> > module. My proposal thus is to: > >> > - Move tests from this suite to relevant suites, comment them out. > >> > - Kill this module (with fire). > >> > > >> > Would be glad to her your input, > >> > > >> > > >> > > >> > -- > >> > Ilya Kasnacheev > >> > > >> > 2018-05-03 20:03 GMT+03:00 Ilya Kasnacheev <ilya.kasnach...@gmail.com > >: > >> > > >> > > Hello Dmitry, igniters! > >> > > > >> > > Still, the policy of removal of unused tests is not clear to me. > >> > > > >> > > We have roughly three groups of such tests: > >> > > - Odd ancient main class tests. I think we can remove those. > >> > > - JVM features/quirks tests (some are main class, some are JUnit > >> tests. > >> > > Reside in package jvmtest. Should we remove these? > >> > > - JUnit "load" tests. Should we kill all of these? I'm asking since > >> > you've > >> > > commited such test recently. I think you wanted it to linger. And > yet, > >> > > what's our policy? How do I determine whether it's safe to nuke a > >> "load" > >> > > test not in any suite? Or just tuck them in a fake TestSuite and > keep? > >> > > > >> > > Regards, > >> > > > >> > > -- > >> > > Ilya Kasnacheev > >> > > > >> > > 2018-04-24 17:54 GMT+03:00 Dmitry Pavlov <dpavlov....@gmail.com>: > >> > > > >> > >> I agree with Yakov here. If nobody responds here we can consider we > >> have > >> > >> lazy consensus on removal of tests. > >> > >> > >> > >> I'm going to review PRs from Ilya. > >> > >> > >> > >> вт, 24 апр. 2018 г. в 6:11, Yakov Zhdanov <yzhda...@apache.org>: > >> > >> > >> > >> > Alexey Goncharuk, Vladimir Ozerov, what do you think about these > >> > tests? > >> > >> > > >> > >> > I believe they were created as a part of variuos optimization and > >> > >> profiling > >> > >> > activities. I also think we can remove them since nobody cares > >> about > >> > >> them > >> > >> > for too long. > >> > >> > > >> > >> > Thoughts? > >> > >> > > >> > >> > Yakov Zhdanov > >> > >> > > >> > >> > ср, 18 апр. 2018 г., 16:42 Ilya Kasnacheev < > >> ilya.kasnach...@gmail.com > >> > >: > >> > >> > > >> > >> > > Hello! > >> > >> > > > >> > >> > > I've decided to return to this task after a break. > >> > >> > > > >> > >> > > Can you please tell me why do we have main-class tests? Such as > >> > >> > > > >> > >> > > GridBasicPerformanceTest.class, > >> > >> > > GridBenchmarkCacheGetLoadTest.class, > >> > >> > > GridBoundedConcurrentLinkedHashSetLoadTest.class, > >> > >> > > GridCacheDataStructuresLoadTest.class, > >> > >> > > GridCacheReplicatedPreloadUndeploysTest.class, > >> > >> > > GridCacheLoadTest.class, > >> > >> > > GridCacheMultiNodeDataStructureTest.class, > >> > >> > > GridCapacityLoadTest.class, > >> > >> > > GridContinuousOperationsLoadTest.class, > >> > >> > > GridFactoryVmShutdownTest.class, > >> > >> > > GridFutureListenPerformanceTest.class, > >> > >> > > GridFutureQueueTest.class, > >> > >> > > GridGcTimeoutTest.class, > >> > >> > > GridJobExecutionSingleNodeLoadTest.class, > >> > >> > > GridJobExecutionSingleNodeSemaphoreLoadTest.class, > >> > >> > > GridJobLoadTest.class, > >> > >> > > GridMergeSortLoadTest.class, > >> > >> > > GridNioBenchmarkTest.class, > >> > >> > > GridThreadPriorityTest.class, > >> > >> > > GridSystemCurrentTimeMillisTest.class, > >> > >> > > BlockingQueueTest.class, > >> > >> > > MultipleFileIOTest.class, > >> > >> > > GridSingleExecutionTest.class > >> > >> > > > >> > >> > > > >> > >> > > If nobody wants them, how about we delete them in master > branch? > >> > Start > >> > >> > > afresh? > >> > >> > > > >> > >> > > -- > >> > >> > > Ilya Kasnacheev > >> > >> > > > >> > >> > > 2018-02-13 17:02 GMT+03:00 Ilya Kasnacheev < > >> > ilya.kasnach...@gmail.com > >> > >> >: > >> > >> > > > >> > >> > > > Anton, > >> > >> > > > > >> > >> > > > >Tests should be attached to appropriate suites > >> > >> > > > > >> > >> > > > This I can do > >> > >> > > > > >> > >> > > > > and muted if necessary, Issues should be created on each > >> mute. > >> > >> > > > > >> > >> > > > This is roughly a week of work. I can't spare that right > now. I > >> > >> doubt > >> > >> > > > anyone can. > >> > >> > > > > >> > >> > > > Can we approach this by smaller steps? > >> > >> > > > > >> > >> > > > -- > >> > >> > > > Ilya Kasnacheev > >> > >> > > > > >> > >> > > > 2018-02-06 19:55 GMT+03:00 Anton Vinogradov < > >> > >> avinogra...@gridgain.com > >> > >> > >: > >> > >> > > > > >> > >> > > >> Val, > >> > >> > > >> > >> > >> > > >> Tests should be attached to appropriate suites and muted if > >> > >> necessary, > >> > >> > > >> Issues should be created on each mute. > >> > >> > > >> > >> > >> > > >> On Tue, Feb 6, 2018 at 7:23 PM, Valentin Kulichenko < > >> > >> > > >> valentin.kuliche...@gmail.com> wrote: > >> > >> > > >> > >> > >> > > >> > Anton, > >> > >> > > >> > > >> > >> > > >> > I tend to agree with Ilya that identifying and fixing all > >> the > >> > >> > possible > >> > >> > > >> > broken tests in one go is not feasible. What is the proper > >> way > >> > in > >> > >> > your > >> > >> > > >> > view? What are you suggesting? > >> > >> > > >> > > >> > >> > > >> > -Val > >> > >> > > >> > > >> > >> > > >> > On Mon, Feb 5, 2018 at 2:18 AM, Anton Vinogradov < > >> > >> > > >> avinogra...@gridgain.com > >> > >> > > >> > > > >> > >> > > >> > wrote: > >> > >> > > >> > > >> > >> > > >> > > Ilya, > >> > >> > > >> > > > >> > >> > > >> > > 1) Still see no reason for such changes. Does this break > >> > >> > something? > >> > >> > > >> > > > >> > >> > > >> > > 2) Looks like you're trying to add Trash*TestSuite.java > >> which > >> > >> will > >> > >> > > >> never > >> > >> > > >> > be > >> > >> > > >> > > refactored. > >> > >> > > >> > > We should do everything in proper way now, not sometime. > >> > >> > > >> > > > >> > >> > > >> > > 3) Your comments looks odd to me. > >> > >> > > >> > > Issue should be resolved in proper way. > >> > >> > > >> > > > >> > >> > > >> > > On Mon, Feb 5, 2018 at 1:07 PM, Ilya Kasnacheev < > >> > >> > > >> > ilya.kasnach...@gmail.com > >> > >> > > >> > > > > >> > >> > > >> > > wrote: > >> > >> > > >> > > > >> > >> > > >> > > > Anton, > >> > >> > > >> > > > > >> > >> > > >> > > > 1) We already have ~100 files named > >> "*AbstractTest.java". > >> > >> > Renaming > >> > >> > > >> > these > >> > >> > > >> > > > several files will help checking for orphaned tests in > >> the > >> > >> > future, > >> > >> > > >> as > >> > >> > > >> > > well > >> > >> > > >> > > > as increasing code base consistency. > >> > >> > > >> > > > > >> > >> > > >> > > > 2) This is huge work that is not doable by any single > >> > >> developer. > >> > >> > > >> While > >> > >> > > >> > > > IgniteLostAndFoundTestSuite can be slowly refactored > >> away > >> > >> > > >> > > > This is unless you are OK with putting all these > tests, > >> > most > >> > >> of > >> > >> > > >> which > >> > >> > > >> > are > >> > >> > > >> > > > red and some are hanging, in production test suites > and > >> > >> > therefore > >> > >> > > >> > > breaking > >> > >> > > >> > > > productivity for a couple months while this gets > sorted. > >> > >> > > >> > > > Are you OK with that? Anybody else? > >> > >> > > >> > > > > >> > >> > > >> > > > 3) I think I *could* put them in some test suite or > >> > another, > >> > >> but > >> > >> > > I'm > >> > >> > > >> > > pretty > >> > >> > > >> > > > sure I can't fix them all, not in one commit, not > ever. > >> > >> Nobody > >> > >> > can > >> > >> > > >> do > >> > >> > > >> > > that > >> > >> > > >> > > > single-handedly. We need a plan here. > >> > >> > > >> > > > > >> > >> > > >> > > > Ilya. > >> > >> > > >> > > > > >> > >> > > >> > > > > >> > >> > > >> > > > -- > >> > >> > > >> > > > Ilya Kasnacheev > >> > >> > > >> > > > > >> > >> > > >> > > > 2018-02-05 13:00 GMT+03:00 Anton Vinogradov < > >> > >> > > >> avinogra...@gridgain.com > >> > >> > > >> > >: > >> > >> > > >> > > > > >> > >> > > >> > > > > Ilya, > >> > >> > > >> > > > > > >> > >> > > >> > > > > 1) I don't think it's a good idea to rename classes > to > >> > >> > > >> > > *AbstractTest.java > >> > >> > > >> > > > > since they already have abstract word at definition. > >> > >> > > >> > > > > We can perform such renaming only in case whole > >> project > >> > >> will > >> > >> > be > >> > >> > > >> > > > refactored, > >> > >> > > >> > > > > but I see no reason to do this. > >> > >> > > >> > > > > > >> > >> > > >> > > > > 2) All not included test should be included to > >> > appropriate > >> > >> > > siutes. > >> > >> > > >> > > > > Creating IgniteLostAndFoundTestSuite,java is not > >> > >> acceptable. > >> > >> > > >> > > > > > >> > >> > > >> > > > > 3) In case you're not sure what to do with > particular > >> > >> tests, > >> > >> > > >> please > >> > >> > > >> > > > provide > >> > >> > > >> > > > > lists of such tests. Please group tests by > "problem". > >> > >> > > >> > > > > > >> > >> > > >> > > > > > >> > >> > > >> > > > > On Fri, Feb 2, 2018 at 12:28 AM, Dmitry Pavlov < > >> > >> > > >> > dpavlov....@gmail.com> > >> > >> > > >> > > > > wrote: > >> > >> > > >> > > > > > >> > >> > > >> > > > > > Hi Ilya, > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > Thank you for this research. I think it is useful > >> for > >> > >> > > community > >> > >> > > >> to > >> > >> > > >> > > > > identify > >> > >> > > >> > > > > > and remove obsolete tests (if any), and include > lost > >> > test > >> > >> > into > >> > >> > > >> CI > >> > >> > > >> > run > >> > >> > > >> > > > > chain > >> > >> > > >> > > > > > (if applicable). > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > For test with main() methods I suggest to ask > >> authors > >> > >> (git > >> > >> > > >> > annotate) > >> > >> > > >> > > > and > >> > >> > > >> > > > > if > >> > >> > > >> > > > > > there is no response probably we should remove > such > >> > code. > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > Since I am not sure all tests in this lost&found > >> suite > >> > >> are > >> > >> > > quite > >> > >> > > >> > > > stable I > >> > >> > > >> > > > > > suggest to create standalone TC Run configuration > >> for > >> > >> such > >> > >> > > >> tests. > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > Earlier I've removed most of tests causing > timeouts > >> > from > >> > >> > basic > >> > >> > > >> > suite. > >> > >> > > >> > > > > > Ideally Basic suite should contain fast run quite > >> > stable > >> > >> > > tests ( > >> > >> > > >> > and > >> > >> > > >> > > 0 > >> > >> > > >> > > > > > flaky ) because it is included into RunAllBasic > sub > >> set > >> > >> to > >> > >> > > brief > >> > >> > > >> > > commit > >> > >> > > >> > > > > > check ( > >> > >> > > >> > > > > > https://ci.ignite.apache.org/ > >> > viewType.html?buildTypeId= > >> > >> > > >> > > > > IgniteTests24Java8_ > >> > >> > > >> > > > > > RunBasicTests > >> > >> > > >> > > > > > ). > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > Sincerely, > >> > >> > > >> > > > > > Dmitriy Pavlov > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > чт, 1 февр. 2018 г. в 20:22, Ilya Kasnacheev < > >> > >> > > >> > > > ilya.kasnach...@gmail.com > >> > >> > > >> > > > > >: > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > > Hello! > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > While working on Ignite, I have noticed that not > >> all > >> > >> tests > >> > >> > > >> are in > >> > >> > > >> > > any > >> > >> > > >> > > > > > test > >> > >> > > >> > > > > > > suite, hence I expect they are ignored. I have > >> also > >> > >> > noticed > >> > >> > > >> some > >> > >> > > >> > > > files > >> > >> > > >> > > > > in > >> > >> > > >> > > > > > > src/test and named *Test.java are actually > >> runnable > >> > >> > > >> main-classes > >> > >> > > >> > > and > >> > >> > > >> > > > > not > >> > >> > > >> > > > > > > tests. I think they're ignored to. Also I've > >> noticed > >> > >> that > >> > >> > 6 > >> > >> > > >> tests > >> > >> > > >> > > > > repeat > >> > >> > > >> > > > > > > twice. > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > I have tried to fix it by introducing "lost and > >> > found" > >> > >> > test > >> > >> > > >> > suite. > >> > >> > > >> > > > Not > >> > >> > > >> > > > > > sure > >> > >> > > >> > > > > > > what to do with main-classes. I have also > renamed > >> > >> abstract > >> > >> > > >> test > >> > >> > > >> > > > classes > >> > >> > > >> > > > > > to > >> > >> > > >> > > > > > > *AbstractTest. > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > Please consider pull request > >> > >> https://github.com/apache/ > >> > >> > > >> > > > > ignite/pull/3464 > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > I have started this suite on TC but I expect it > to > >> > >> hang or > >> > >> > > >> worse. > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > > >> > >> > https://ci.ignite.apache.org/viewLog.html?buildId=1071504&; > >> > >> > > >> > > > > > tab=queuedBuildOverviewTab > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > Regards, > >> > >> > > >> > > > > > > -- > >> > >> > > >> > > > > > > Ilya Kasnacheev > >> > >> > > >> > > > > > > > >> > >> > > >> > > > > > > >> > >> > > >> > > > > > >> > >> > > >> > > > > >> > >> > > >> > > > >> > >> > > >> > > >> > >> > > >> > >> > >> > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > > > >> > > > >> > > >> > > >
