Hello! Just to resurrect this old thread:
I have uncommented another batch of tests, would appreciate a review of PR: https://issues.apache.org/jira/browse/IGNITE-9216 Regards, -- Ilya Kasnacheev ср, 31 окт. 2018 г. в 15:22, Ilya Kasnacheev <ilya.kasnach...@gmail.com>: > Hello! > > So we have uncommented 4 batches out of 10! 6 to go. Some broken > functionality were exposed. > > There is still work to do, so do not hesitate to assign a subtask to > yourself. > > Regards, > -- > Ilya Kasnacheev > > > ср, 15 авг. 2018 г. в 19:42, Ilya Kasnacheev <ilya.kasnach...@gmail.com>: > >> Hello! >> >> So we have enabled a first batch of tests: >> https://github.com/apache/ignite/pull/4504 >> >> How it was done: I have uncommented classes. Some of these were absent in >> code base, so I have checked if we didn't lose anything important - they >> were testing CLOCK mode which isn't with us for some time, so I removed >> their entries. >> Then I have ran them, some were broken. Most of those were testing >> on-heap caching with copy=false, which now requires setOnheapCaching(true), >> which I did. After that, cache.invoke() still didn't work, so I commented >> this part out. >> The remaining test was broken due to dependence on hash map iteration >> order, which was changed in Java 8. So I have got the remaining tests >> working, checking important parts of our system. >> >> Please do not hesitate to assign subtasks of >> https://issues.apache.org/jira/browse/IGNITE-9210 to yourself, dabble >> with tests. IMO it's the best way for a novice developer to become >> acquainted with Ignite code base, tests and history, while helping the >> project. >> >> Thanks, >> >> -- >> Ilya Kasnacheev >> >> 2018-08-07 16:54 GMT+03:00 Ilya Kasnacheev <ilya.kasnach...@gmail.com>: >> >>> 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 >>>> >> > >> > > >> > > > > > > >>>> >> > >> > > >> > > > > > >>>> >> > >> > > >> > > > > >>>> >> > >> > > >> > > > >>>> >> > >> > > >> > > >>>> >> > >> > > >> > >>>> >> > >> > > >> >>>> >> > >> > > > >>>> >> > >> > > > >>>> >> > >> > > >>>> >> > >> > >>>> >> > >> >>>> >> > > >>>> >> > > >>>> >> > >>>> >> >>>> > >>>> >>> >>> >>
