ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ilya Kasnacheev <ilya.kasnach...@gmail.com>
Subject Re: [DISCUSS] Missed (non-suited) tests
Date Wed, 25 Nov 2020 13:26:25 GMT
Hello!

Yes, we have such tests which depend on ignite-indexing or ignite-spring.
They just need to be included in Spring* or Queries* test suite. Then they
will be executed on TC in the correct context. You can also run these tests
from IDEA by specifying other module as classpath. No need to move the
classes around.

I will check the PR.

Regards,
-- 
Ilya Kasnacheev


ср, 25 нояб. 2020 г. в 00:22, Max Timonin <timonin.maxim@gmail.com>:

> Ilya, Anton, Ivan, hi!
>
> I fix some comments you leave in the PR. Also I checked some test suites
> and found that some tests are written in the core module but depend on the
> indexing module (or other modules). Some of such test classes contain tests
> that are related to the core functionality, but some to indexing. I'm not
> sure if it is correct to move a whole suite with all tests from the
> indexing module to the core, as it will hide some core tests from the core
> module.
>
> I believe that the correct solution is to investigate every such test and
> move it to the right module. But I think this work will take a lot of time
> and should be performed in a separate ticket, I will do it in the
> background.
>
> I think currently we should proceed with a way I introduced in PR:
> 1. Create fake suites for all such tests (written in core, suited in other
> modules: indexing/spring/zookeeper/etc) in the core module. I named such
> suites with prefix Core*.
> 2. Replace tests in modules with links to fake suites.
> 3. Create an umbrella Jira ticket to discover every fake suite and replace
> it with a new one in the right module.
> 4. Merge this PR for introducing a new travis check to avoid losing
> new tests.
>
> WDYT?
>
> List of such mixed suites:
>
> 1. suite IgniteBinaryCacheQueryTestSuite
>
> test GridCacheQueryIndexingDisabledSelfTest
> test IgniteCacheBinaryObjectsScanSelfTest
> test IgniteCacheBinaryObjectsScanWithEventsSelfTest)
>
>
> 2. suite IgniteCacheQuerySelfTestSuite3
>
> test GridCacheContinuousQueryNodesFilteringTest
>
>
> 3. suite IgniteCacheQuerySelfTestSuite5
>
> test ContinuousQueryRemoteFilterMissingInClassPathSelfTest
>
> 4. suite IgniteCacheQuerySelfTestSuite6
>
> test CacheContinuousQueryOperationP2PTest
>
> test CacheContinuousQueryFilterDeploymentFailedTest
>
>
> 5. all tests in suite IgnitePdsWithIndexingCoreTestSuite
>
>
> 6. and some others.
>
> On Wed, Nov 18, 2020 at 12:38 PM Max Timonin <timonin.maxim@gmail.com>
> wrote:
>
> > Hi Ilya! Thank you for up the topic. I will come back with fixes and
> > comments in a couple of days.
> >
> > On Tue, Nov 17, 2020 at 4:26 PM Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com>
> > wrote:
> >
> >> Hello!
> >>
> >> I have left some comments and there's also more discussion there. Can
> you
> >> please look?
> >>
> >> Thanks,
> >> --
> >> Ilya Kasnacheev
> >>
> >>
> >> вт, 3 нояб. 2020 г. в 00:03, Max Timonin <timonin.maxim@gmail.com>:
> >>
> >> > 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