I suggests to mark these tests with @Ignore and file tickets to fix them.

пт, 30 окт. 2020 г. в 16:26, Ivan Daschinsky <ivanda...@gmail.com>:

> Hi
>
> WalCompactionAfterRestartTest -- yes we need it. This test failed because
> of race (test shold be rewritten a little bit)
>
> пт, 30 окт. 2020 г. в 16:15, Max Timonin <timonin.ma...@gmail.com>:
>
>> 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
>> > >
>> >
>>
>
>
> --
> Sincerely yours, Ivan Daschinskiy
>


-- 
Sincerely yours, Ivan Daschinskiy

Reply via email to