ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Max Timonin <timonin.ma...@gmail.com>
Subject Re: [DISCUSS] Missed (non-suited) tests
Date Mon, 02 Nov 2020 21:02:58 GMT
Hi!

I've updated PR: https://github.com/apache/ignite/pull/8367. Anton, Ivan,
Ivan could you please review it?

Some moments to mention:
1. I've added new suites: SerializerSuite (ignite-cassandra-serializers),
DistanceTestSuite, NaiveBayesTestSuite (ignite-ml). Should we configure a
TeamCity to run them?

2. Some tests marked as failed, I'll create corresponding tickets for them
after PR approved:
- IgnitePKIndexesMigrationToUnwrapPkTest
- P2PGridifySelfTest
- GridCacheMultithreadedFailoverAbstractTest
- WalCompactionAfterRestartTest
- GridTcpCommunicationSpiLogTest
- ComplexSecondaryKeyUnwrapSelfTest
- SqlTransactionsSelfTest

3. Add docs to DEVNOTES.txt

On Mon, Nov 2, 2020 at 11:44 AM Anton Vinogradov <av@apache.org> wrote:

> > As I understand we
> > can't just move suites between modules, as TeamCity may depend on the
> path
> > to them.
> See no problem to update TC as well.
>
> On Fri, Oct 30, 2020 at 4:32 PM Ivan Daschinsky <ivandasch@gmail.com>
> wrote:
>
> > I suggests to mark these tests with @Ignore and file tickets to fix them.
> >
> > пт, 30 окт. 2020 г. в 16:26, Ivan Daschinsky <ivandasch@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.maxim@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 <av@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 <
> vololo100@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.maxim@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 <
> > >> ivandasch@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 <
> vololo100@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.maxim@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.stepachev@gmail.com:
> > >> > > >> > >         GridTcpCommunicationSpiLogTest
> > >> > > >> > >
> > >> > > >> > > nizhikov@apache.org:
> > >> > > >> > >         IgniteCacheClientMultiNodeUpdateTopologyLockTest
> > >> > > >> > >         CacheClientsConcurrentStartTest
> > >> > > >> > >         IgniteOutOfMemoryPropagationTest
> > >> > > >> > >         GridCacheP2PUndeploySelfTest
> > >> > > >> > >         GridCacheRebalancingOrderingTest
> > >> > > >> > >         IgniteMassLoadSandboxTest
> > >> > > >> > >         PageLockTrackerMXBeanImplTest
> > >> > > >> > >         IgniteBinaryMetadataUpdateNodeRestartTest
> > >> > > >> > >         CacheLockCandidatesThreadTest
> > >> > > >> > >         GridMBeanBaselineTest
> > >> > > >> > >         RendezvousAffinityFunctionSimpleBenchmark
> > >> > > >> > >
> > >> > > >> > > samvimes@yandex.ru:
> > >> > > >> > >         IgnitePdsNoSpaceLeftOnDeviceTest
> > >> > > >> > >
> > >> > > >> > > maxmuzaf@gmail.com:
> > >> > > >> > >         GridCacheOnCopyFlagReplicatedSelfTest
> > >> > > >> > >         GridCacheOnCopyFlagLocalSelfTest
> > >> > > >> > >         GridCacheReplicatedAtomicReferenceMultiNodeTest
> > >> > > >> > >         GridCacheReplicatedMarshallerTxTest
> > >> > > >> > >         GridCacheReplicatedTxConcurrentGetTest
> > >> > > >> > >         GridCacheOnCopyFlagTxPartitionedSelfTest
> > >> > > >> > >         GridCacheReplicatedTxReadTest
> > >> > > >> > >         GridCachePartitionedAtomicReferenceMultiNodeTest
> > >> > > >> > >         GridCacheOnCopyFlagAtomicSelfTest
> > >> > > >> > >
> > >> > > >> > > mmuzaf@apache.org:
> > >> > > >> > >         GridActivateExtensionTest
> > >> > > >> > >         IgniteChangeGlobalStateCacheTest
> > >> > > >> > >         IgniteChangeGlobalStateTest
> > >> > > >> > >         IgniteChangeGlobalStateServiceTest
> > >> > > >> > >         IgniteChangeGlobalStateDataStructureTest
> > >> > > >> > >
> > >> > > >> > > oignatenko@gridgain.com:
> > >> > > >> > >         CacheEntryProcessorCopySelfTest
> > >> > > >> > >         MemoryLeaksOnRestartNodeTest
> > >> > > >> > >         GridCacheAtomicPreloadSelfTest
> > >> > > >> > >         WalCompactionAfterRestartTest
> > >> > > >> > >         IgniteCacheConcurrentPutGetRemove
> > >> > > >> > >         GridIoManagerBenchmark0
> > >> > > >> > >
> > >> > > >> > > nsamelchev@gmail.com:
> > >> > > >> > >         GridLongRunningInitNewCrdFutureDiagnosticsTest
> > >> > > >> > >         GridCacheMultithreadedFailoverAbstractTest
> > >> > > >> > >
> > >> > > >> > > alexey.goncharuk@gmail.com:
> > >> > > >> > >         GridCacheBinaryObjectsAtomicOnheapSelfTest
> > >> > > >> > >
> >  GridCacheBinaryObjectsAtomicNearDisabledOnheapSelfTest
> > >> > > >> > >         GridCacheBinaryObjectsPartitionedOnheapSelfTest
> > >> > > >> > >
> > >> >  GridCacheBinaryObjectsPartitionedNearDisabledOnheapSelfTest
> > >> > > >> > >
> > >> > > >> > > vladisavj@gmail.com:
> > >> > > >> > >         IgnitePartitionedLockSelfTest
> > >> > > >> > >
> > >> > > >> > > alexandr.belyak@xored.com:
> > >> > > >> > >         IgniteStableBaselineCachePutAllFailoverTest
> > >> > > >> > >         IgniteStableBaselineCacheRemoveFailoverTest
> > >> > > >> > >
> > >> > > >> > > ilantukh@gridgain.com:
> > >> > > >> > >         IgniteCacheAtomicOnheapExpiryPolicyTest
> > >> > > >> > >         IgniteCacheAtomicLocalOnheapExpiryPolicyTest
> > >> > > >> > >         GridCacheReplicatedOnheapFullApiSelfTest
> > >> > > >> > >         GridCacheBinaryObjectsLocalOnheapSelfTest
> > >> > > >> > >
> > >> > > >> > > oignatenko@users.noreply.github.com:
> > >> > > >> > >         GridCacheTtlManagerEvictionSelfTest
> > >> > > >> > >
> > >> > > >> > > irakov@apache.org:
> > >> > > >> > >         CommonPoolStarvationCheckpointTest
> > >> > > >> > >
> > >> > > >> > > alievmirza@gmail.com:
> > >> > > >> > >         RemoveAllDeadlockTest
> > >> > > >> > >
> > >> > > >> > > schugunov@gridgain.com:
> > >> > > >> > >         FullyConnectedComponentSearcherTest
> > >> > > >> > >
> > >> > > >> > > sboikov@gridgain.com:
> > >> > > >> > >         IgniteDataStructuresNoClassOnServerTest
> > >> > > >> > >
> > >> > > >> > > timonin.maxim@gmail.com:
> > >> > > >> > >         ReliableChannelTest
> > >> > > >> > >         ThinClientPartitionAwarenessDiscoveryTest
> > >> > > >> > >
> > >> > > >> >
> > >> > > >> >
> > >> > > >> > --
> > >> > > >> >
> > >> > > >> > Best regards,
> > >> > > >> > Ivan Pavlukhin
> > >> > > >> >
> > >> > > >>
> > >> > > >>
> > >> > > >> --
> > >> > > >> Sincerely yours, Ivan Daschinskiy
> > >> > > >>
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > >
> > >> > > Best regards,
> > >> > > Ivan Pavlukhin
> > >> > >
> > >> >
> > >>
> > >
> > >
> > > --
> > > Sincerely yours, Ivan Daschinskiy
> > >
> >
> >
> > --
> > Sincerely yours, Ivan Daschinskiy
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message