Hi! Yes, you're correct. I've developed the check and started to clean tests (move them to suites, mark some tests with Ignore, etc.). I finish work on the core module. I hope it was the biggest one, and others are less. If so, I think I will finish the work on other modules in 1 or 2 weeks, as I do this activity in the background (~10% of my work time). Actually I've found 3 failed tests in the core module that aren't in any suite, so I need time to discover reason of failures and if we actually need those tests:
GridCacheMultithreadedFailoverAbstractTest WalCompactionAfterRestartTest GridTcpCommunicationSpiLogTest Also we should decide how to be with wrongly located es. As I understand we can't just move suites between modules, as TeamCity may depend on the path to them. So, for such cases I've just created suites in the right module, and replaced the test list with the new class suite. It does not look pretty enough, but I think It's a path of least resistance. WDYT? BEFORE: Module A -> SuiteA -> testA1, testA2, testB1, testB2 Module B -> testB1, testB2 AFTER: Module A -> SuiteA, SuiteB Module B -> SuiteB -> testB1, testB2 On Fri, Oct 30, 2020 at 3:38 PM Anton Vinogradov <a...@apache.org> wrote: > Folks, > What's the current state of this thread? > AFAIU, we found unused and wrongly located tests and developed some > checker, could we split this to some PRs? > Let's merge tests usage fix and location fixes first, this will provide us > an ability to automate check using Travis. > > On Tue, Oct 20, 2020 at 12:06 PM Ivan Pavlukhin <vololo...@gmail.com> > wrote: > > > Max, Ivan, > > > > Using explicit @Ignore and the automated check sounds good to me. If > > nobody has arguments against it I think we should do it. > > > > 2020-10-19 19:30 GMT+03:00, Max Timonin <timonin.ma...@gmail.com>: > > > Hi Ivan, > > > > > > I've checked the ticket you provide. It contains subtasks to uncomment > or > > > to remove some unused tests. It definitely describes some cases I've > > found. > > > So what do you think if I uncomment them in suites, add @Ignore > > annotation > > > for those tests while the tickets are open? This will help to find out > > > tests that were forgiven in a recent time. > > > > > > Also I believe that this check must be automated. I didn't find a way > how > > > uncomment / unused tests are found in the ticket. If there is no any - > I > > > propose mine PR for this purpose. > > > > > > > > > > > > On Mon, Oct 19, 2020 at 5:24 PM Ivan Daschinsky <ivanda...@gmail.com> > > > wrote: > > > > > >> Ivan, as far as I understand, Max also created verification check for > > not > > >> included test and found a few tests, that have never been included in > > any > > >> testsuites. > > >> > > >> Also, I suppose, that even if we cannot run some tests, these tests > > >> should > > >> be ignored using annotation, but not commented. > > >> > > >> пн, 19 окт. 2020 г. в 16:33, Ivan Pavlukhin <vololo...@gmail.com>: > > >> > > >> > Hi Max, > > >> > > > >> > There is an existing effort about "abandoned" tests > > >> > https://issues.apache.org/jira/browse/IGNITE-9210 > > >> > > > >> > 2020-10-19 16:25 GMT+03:00, Max Timonin <timonin.ma...@gmail.com>: > > >> > > Hi Igniters! > > >> > > > > >> > > I made a research into tests that aren't included in any test > suite. > > >> > > As > > >> > > TeamCity runs tests by suites so there could be tests that never > run > > >> > > on > > >> > TC. > > >> > > So I tried implementing a simple check for such tests and include > it > > >> > > in > > >> > > Ignite's travis config. > > >> > > > > >> > > The check runs while "mvn test" command and piggy-backs on the > maven > > >> > > surefire plugin. I replaced the junit provider with a custom one > > that > > >> > > checks if a class is a test or a suite (there are some Ignite > > >> > > specific > > >> > > stuff), marks tests that are in suites and raises an exception if > > >> > > there > > >> > are > > >> > > non-suited tests. It's implemented as a part of maven command so > it > > >> runs > > >> > > for every module separately. > > >> > > > > >> > > I've prepared draft PR with this check: > > >> > > https://github.com/apache/ignite/pull/8367 > > >> > > Travis check report is here: > > >> > > https://travis-ci.org/github/apache/ignite/jobs/737046387 > > >> > > As It's a draft, so I skip some maven configuration steps for a > > >> > > while. > > >> > Also > > >> > > I run the check only for the core module. > > >> > > > > >> > > But I have some results that want to discuss before continue the > > >> > > work: > > >> > > 1. Currently in the core module there are 53 tests that aren't > part > > >> > > of > > >> > any > > >> > > test suite. I'm not sure about the reason for every test. So I > just > > >> > > put > > >> > > below a list of the tests and last contributor to a file that > > >> > > contains > > >> a > > >> > > test. > > >> > > > > >> > > 2. Some tests are located in the core module, but suites are in a > > >> > > different, for example ignite-indexing suite > > >> > > IgniteCacheQuerySelfTestSuite3 contains > > >> > > only tests written in the core module, and none from the indexing > > >> module. > > >> > > Also there are suites in spring, uri-deploy, zookeeper modules. In > > my > > >> PR > > >> > > I've just copied the test suites to the core module. > > >> > > > > >> > > 3. Some test classes are named with the "Abstract" suffix but > don't > > >> have > > >> > > the corresponding modifier (for example, > > >> > > IgniteTxTimeoutAbstractTest). > > >> > So, > > >> > > I add the modifier for every such file if it's not a part of any > > >> > > suite. > > >> > > > > >> > > What do you think about this check? If Ignite needs it, let's > > discuss > > >> > next > > >> > > things: > > >> > > 1. Mark tests that should never be in any suite by some reason; > > >> > > 2. Fix the missed tests; > > >> > > 3. How to declare suites that contains tests from a different > > module; > > >> > > 4. How to check if TC runs all suites. > > >> > > > > >> > > List of non-suited tests in the core module: > > >> > > > > >> > > maksim.stepac...@gmail.com: > > >> > > GridTcpCommunicationSpiLogTest > > >> > > > > >> > > nizhi...@apache.org: > > >> > > IgniteCacheClientMultiNodeUpdateTopologyLockTest > > >> > > CacheClientsConcurrentStartTest > > >> > > IgniteOutOfMemoryPropagationTest > > >> > > GridCacheP2PUndeploySelfTest > > >> > > GridCacheRebalancingOrderingTest > > >> > > IgniteMassLoadSandboxTest > > >> > > PageLockTrackerMXBeanImplTest > > >> > > IgniteBinaryMetadataUpdateNodeRestartTest > > >> > > CacheLockCandidatesThreadTest > > >> > > GridMBeanBaselineTest > > >> > > RendezvousAffinityFunctionSimpleBenchmark > > >> > > > > >> > > samvi...@yandex.ru: > > >> > > IgnitePdsNoSpaceLeftOnDeviceTest > > >> > > > > >> > > maxmu...@gmail.com: > > >> > > GridCacheOnCopyFlagReplicatedSelfTest > > >> > > GridCacheOnCopyFlagLocalSelfTest > > >> > > GridCacheReplicatedAtomicReferenceMultiNodeTest > > >> > > GridCacheReplicatedMarshallerTxTest > > >> > > GridCacheReplicatedTxConcurrentGetTest > > >> > > GridCacheOnCopyFlagTxPartitionedSelfTest > > >> > > GridCacheReplicatedTxReadTest > > >> > > GridCachePartitionedAtomicReferenceMultiNodeTest > > >> > > GridCacheOnCopyFlagAtomicSelfTest > > >> > > > > >> > > mmu...@apache.org: > > >> > > GridActivateExtensionTest > > >> > > IgniteChangeGlobalStateCacheTest > > >> > > IgniteChangeGlobalStateTest > > >> > > IgniteChangeGlobalStateServiceTest > > >> > > IgniteChangeGlobalStateDataStructureTest > > >> > > > > >> > > oignate...@gridgain.com: > > >> > > CacheEntryProcessorCopySelfTest > > >> > > MemoryLeaksOnRestartNodeTest > > >> > > GridCacheAtomicPreloadSelfTest > > >> > > WalCompactionAfterRestartTest > > >> > > IgniteCacheConcurrentPutGetRemove > > >> > > GridIoManagerBenchmark0 > > >> > > > > >> > > nsamelc...@gmail.com: > > >> > > GridLongRunningInitNewCrdFutureDiagnosticsTest > > >> > > GridCacheMultithreadedFailoverAbstractTest > > >> > > > > >> > > alexey.goncha...@gmail.com: > > >> > > GridCacheBinaryObjectsAtomicOnheapSelfTest > > >> > > GridCacheBinaryObjectsAtomicNearDisabledOnheapSelfTest > > >> > > GridCacheBinaryObjectsPartitionedOnheapSelfTest > > >> > > > GridCacheBinaryObjectsPartitionedNearDisabledOnheapSelfTest > > >> > > > > >> > > vladis...@gmail.com: > > >> > > IgnitePartitionedLockSelfTest > > >> > > > > >> > > alexandr.bel...@xored.com: > > >> > > IgniteStableBaselineCachePutAllFailoverTest > > >> > > IgniteStableBaselineCacheRemoveFailoverTest > > >> > > > > >> > > ilant...@gridgain.com: > > >> > > IgniteCacheAtomicOnheapExpiryPolicyTest > > >> > > IgniteCacheAtomicLocalOnheapExpiryPolicyTest > > >> > > GridCacheReplicatedOnheapFullApiSelfTest > > >> > > GridCacheBinaryObjectsLocalOnheapSelfTest > > >> > > > > >> > > oignate...@users.noreply.github.com: > > >> > > GridCacheTtlManagerEvictionSelfTest > > >> > > > > >> > > ira...@apache.org: > > >> > > CommonPoolStarvationCheckpointTest > > >> > > > > >> > > alievmi...@gmail.com: > > >> > > RemoveAllDeadlockTest > > >> > > > > >> > > schugu...@gridgain.com: > > >> > > FullyConnectedComponentSearcherTest > > >> > > > > >> > > sboi...@gridgain.com: > > >> > > IgniteDataStructuresNoClassOnServerTest > > >> > > > > >> > > timonin.ma...@gmail.com: > > >> > > ReliableChannelTest > > >> > > ThinClientPartitionAwarenessDiscoveryTest > > >> > > > > >> > > > >> > > > >> > -- > > >> > > > >> > Best regards, > > >> > Ivan Pavlukhin > > >> > > > >> > > >> > > >> -- > > >> Sincerely yours, Ivan Daschinskiy > > >> > > > > > > > > > -- > > > > Best regards, > > Ivan Pavlukhin > > >
