ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: [DISCUSS] Missed (non-suited) tests
Date Mon, 02 Nov 2020 08:43:54 GMT
> 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