ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Pavlov <dpavlov....@gmail.com>
Subject Re: Orphaned, duplicate, and main-class tests!
Date Tue, 07 Aug 2018 10:58:06 GMT
Hi Igniters,

I've merged chages for following tickets
IGNITE-7615: Find orphaned tests without test suites, create separate test
suite for them;
IGNITE-8344: Remove duplicate tests and suites;
IGNITE-8345: Streamline tests' class names: mark Abstract and Load tests
obviously so;

After including these suites we have now more than 100 occurrences of
//suite.addTest

These tests were created early but not executed on TeamCity. If you are
interseted in test coverage increase and can contribute each of these suite
actualization, please feel free to create ticket for such suites
resurrection (or group of suites).

Ilya, thank you for contribution and for your efforts to make this happen.

Sincerely,
Dmitriy Pavlov

ср, 1 авг. 2018 г. в 12:52, Dmitriy Pavlov <dpavlov.spb@gmail.com>:

> Hi Ilya,
>
> could you please actualize this PR. TC Bot can now detect newly
> contributed tests' failures, so I think it is best point to apply you
> change.
>
> Sincerely,
> Dmitriy Pavlov
>
> пт, 25 мая 2018 г. в 18:16, Eduard Shangareev <eduard.shangareev@gmail.com
> >:
>
>> Igniters,
>>
>> While making review I checked next main-method tests:
>>
>> org.apache.ignite.loadtests.mapper.GridContinuousMapperLoadTest1
>> org.apache.ignite.loadtests.mapper.GridContinuousMapperLoadTest2
>>
>> And I have found that they are totally outdated!
>> They use config which was changed a long time ago.
>> And use localPeek with parameters which don't make sense now.
>>
>> So, I suggest to delete them.
>>
>> If there wouldn't be any objection I will do it myself.
>>
>>
>>
>>
>> On Tue, May 22, 2018 at 6:59 PM, Ilya Kasnacheev <
>> ilya.kasnacheev@gmail.com>
>> wrote:
>>
>> > Hello, Igniters!
>> >
>> > One moment more of your time. One, we seem to have a consensus now that
>> > tests should be added to suites, but commented out. They should be
>> > uncommented out later, for which numerous tickets will be created. This
>> way
>> > we can tackle.
>> >
>> > Another issue sprang up, just now I have discovered an 'ignored-tests'
>> > module. My proposal thus is to:
>> > - Move tests from this suite to relevant suites, comment them out.
>> > - Kill this module (with fire).
>> >
>> > Would be glad to her your input,
>> >
>> >
>> >
>> > --
>> > Ilya Kasnacheev
>> >
>> > 2018-05-03 20:03 GMT+03:00 Ilya Kasnacheev <ilya.kasnacheev@gmail.com>:
>> >
>> > > Hello Dmitry, igniters!
>> > >
>> > > Still, the policy of removal of unused tests is not clear to me.
>> > >
>> > > We have roughly three groups of such tests:
>> > > - Odd ancient main class tests. I think we can remove those.
>> > > - JVM features/quirks tests (some are main class, some are JUnit
>> tests.
>> > > Reside in package jvmtest. Should we remove these?
>> > > - JUnit "load" tests. Should we kill all of these? I'm asking since
>> > you've
>> > > commited such test recently. I think you wanted it to linger. And yet,
>> > > what's our policy? How do I determine whether it's safe to nuke a
>> "load"
>> > > test not in any suite? Or just tuck them in a fake TestSuite and keep?
>> > >
>> > > Regards,
>> > >
>> > > --
>> > > Ilya Kasnacheev
>> > >
>> > > 2018-04-24 17:54 GMT+03:00 Dmitry Pavlov <dpavlov.spb@gmail.com>:
>> > >
>> > >> I agree with Yakov here. If nobody responds here we can consider we
>> have
>> > >> lazy consensus on removal of tests.
>> > >>
>> > >> I'm going to review PRs from Ilya.
>> > >>
>> > >> вт, 24 апр. 2018 г. в 6:11, Yakov Zhdanov <yzhdanov@apache.org>:
>> > >>
>> > >> > Alexey Goncharuk, Vladimir Ozerov, what do you think about these
>> > tests?
>> > >> >
>> > >> > I believe they were created as a part of variuos optimization
and
>> > >> profiling
>> > >> > activities. I also think we can remove them since nobody cares
>> about
>> > >> them
>> > >> > for too long.
>> > >> >
>> > >> > Thoughts?
>> > >> >
>> > >> > Yakov Zhdanov
>> > >> >
>> > >> > ср, 18 апр. 2018 г., 16:42 Ilya Kasnacheev <
>> ilya.kasnacheev@gmail.com
>> > >:
>> > >> >
>> > >> > > Hello!
>> > >> > >
>> > >> > > I've decided to return to this task after a break.
>> > >> > >
>> > >> > > Can you please tell me why do we have main-class tests? Such
as
>> > >> > >
>> > >> > > GridBasicPerformanceTest.class,
>> > >> > >     GridBenchmarkCacheGetLoadTest.class,
>> > >> > >     GridBoundedConcurrentLinkedHashSetLoadTest.class,
>> > >> > >     GridCacheDataStructuresLoadTest.class,
>> > >> > >     GridCacheReplicatedPreloadUndeploysTest.class,
>> > >> > >     GridCacheLoadTest.class,
>> > >> > >     GridCacheMultiNodeDataStructureTest.class,
>> > >> > >     GridCapacityLoadTest.class,
>> > >> > >     GridContinuousOperationsLoadTest.class,
>> > >> > >     GridFactoryVmShutdownTest.class,
>> > >> > >     GridFutureListenPerformanceTest.class,
>> > >> > >     GridFutureQueueTest.class,
>> > >> > >     GridGcTimeoutTest.class,
>> > >> > >     GridJobExecutionSingleNodeLoadTest.class,
>> > >> > >     GridJobExecutionSingleNodeSemaphoreLoadTest.class,
>> > >> > >     GridJobLoadTest.class,
>> > >> > >     GridMergeSortLoadTest.class,
>> > >> > >     GridNioBenchmarkTest.class,
>> > >> > >     GridThreadPriorityTest.class,
>> > >> > >     GridSystemCurrentTimeMillisTest.class,
>> > >> > >     BlockingQueueTest.class,
>> > >> > >     MultipleFileIOTest.class,
>> > >> > >     GridSingleExecutionTest.class
>> > >> > >
>> > >> > >
>> > >> > > If nobody wants them, how about we delete them in master
branch?
>> > Start
>> > >> > > afresh?
>> > >> > >
>> > >> > > --
>> > >> > > Ilya Kasnacheev
>> > >> > >
>> > >> > > 2018-02-13 17:02 GMT+03:00 Ilya Kasnacheev <
>> > ilya.kasnacheev@gmail.com
>> > >> >:
>> > >> > >
>> > >> > > > Anton,
>> > >> > > >
>> > >> > > > >Tests should be attached to appropriate suites
>> > >> > > >
>> > >> > > > This I can do
>> > >> > > >
>> > >> > > > > and muted if necessary, Issues should be created
on each
>> mute.
>> > >> > > >
>> > >> > > > This is roughly a week of work. I can't spare that right
now. I
>> > >> doubt
>> > >> > > > anyone can.
>> > >> > > >
>> > >> > > > Can we approach this by smaller steps?
>> > >> > > >
>> > >> > > > --
>> > >> > > > Ilya Kasnacheev
>> > >> > > >
>> > >> > > > 2018-02-06 19:55 GMT+03:00 Anton Vinogradov <
>> > >> avinogradov@gridgain.com
>> > >> > >:
>> > >> > > >
>> > >> > > >> Val,
>> > >> > > >>
>> > >> > > >> Tests should be attached to appropriate suites and
muted if
>> > >> necessary,
>> > >> > > >> Issues should be created on each mute.
>> > >> > > >>
>> > >> > > >> On Tue, Feb 6, 2018 at 7:23 PM, Valentin Kulichenko
<
>> > >> > > >> valentin.kulichenko@gmail.com> wrote:
>> > >> > > >>
>> > >> > > >> > Anton,
>> > >> > > >> >
>> > >> > > >> > I tend to agree with Ilya that identifying
and fixing all
>> the
>> > >> > possible
>> > >> > > >> > broken tests in one go is not feasible. What
is the proper
>> way
>> > in
>> > >> > your
>> > >> > > >> > view? What are you suggesting?
>> > >> > > >> >
>> > >> > > >> > -Val
>> > >> > > >> >
>> > >> > > >> > On Mon, Feb 5, 2018 at 2:18 AM, Anton Vinogradov
<
>> > >> > > >> avinogradov@gridgain.com
>> > >> > > >> > >
>> > >> > > >> > wrote:
>> > >> > > >> >
>> > >> > > >> > > Ilya,
>> > >> > > >> > >
>> > >> > > >> > > 1) Still see no reason for such changes.
Does this break
>> > >> > something?
>> > >> > > >> > >
>> > >> > > >> > > 2) Looks like you're trying to add Trash*TestSuite.java
>> which
>> > >> will
>> > >> > > >> never
>> > >> > > >> > be
>> > >> > > >> > > refactored.
>> > >> > > >> > > We should do everything in proper way
now, not sometime.
>> > >> > > >> > >
>> > >> > > >> > > 3) Your comments looks odd to me.
>> > >> > > >> > > Issue should be resolved in proper way.
>> > >> > > >> > >
>> > >> > > >> > > On Mon, Feb 5, 2018 at 1:07 PM, Ilya Kasnacheev
<
>> > >> > > >> > ilya.kasnacheev@gmail.com
>> > >> > > >> > > >
>> > >> > > >> > > wrote:
>> > >> > > >> > >
>> > >> > > >> > > > Anton,
>> > >> > > >> > > >
>> > >> > > >> > > > 1) We already have ~100 files named
>> "*AbstractTest.java".
>> > >> > Renaming
>> > >> > > >> > these
>> > >> > > >> > > > several files will help checking
for orphaned tests in
>> the
>> > >> > future,
>> > >> > > >> as
>> > >> > > >> > > well
>> > >> > > >> > > > as increasing code base consistency.
>> > >> > > >> > > >
>> > >> > > >> > > > 2) This is huge work that is not
doable by any single
>> > >> developer.
>> > >> > > >> While
>> > >> > > >> > > > IgniteLostAndFoundTestSuite can be
slowly refactored
>> away
>> > >> > > >> > > > This is unless you are OK with putting
all these tests,
>> > most
>> > >> of
>> > >> > > >> which
>> > >> > > >> > are
>> > >> > > >> > > > red and some are hanging, in production
test suites and
>> > >> > therefore
>> > >> > > >> > > breaking
>> > >> > > >> > > > productivity for a couple months
while this gets sorted.
>> > >> > > >> > > > Are you OK with that? Anybody else?
>> > >> > > >> > > >
>> > >> > > >> > > > 3) I think I *could* put them in
some test suite or
>> > another,
>> > >> but
>> > >> > > I'm
>> > >> > > >> > > pretty
>> > >> > > >> > > > sure I can't fix them all, not in
one commit, not ever.
>> > >> Nobody
>> > >> > can
>> > >> > > >> do
>> > >> > > >> > > that
>> > >> > > >> > > > single-handedly. We need a plan here.
>> > >> > > >> > > >
>> > >> > > >> > > > Ilya.
>> > >> > > >> > > >
>> > >> > > >> > > >
>> > >> > > >> > > > --
>> > >> > > >> > > > Ilya Kasnacheev
>> > >> > > >> > > >
>> > >> > > >> > > > 2018-02-05 13:00 GMT+03:00 Anton
Vinogradov <
>> > >> > > >> avinogradov@gridgain.com
>> > >> > > >> > >:
>> > >> > > >> > > >
>> > >> > > >> > > > > Ilya,
>> > >> > > >> > > > >
>> > >> > > >> > > > > 1) I don't think it's a good
idea to rename classes to
>> > >> > > >> > > *AbstractTest.java
>> > >> > > >> > > > > since they already have abstract
word at definition.
>> > >> > > >> > > > > We can perform such renaming
only in case whole
>> project
>> > >> will
>> > >> > be
>> > >> > > >> > > > refactored,
>> > >> > > >> > > > > but I see no reason to do this.
>> > >> > > >> > > > >
>> > >> > > >> > > > > 2) All not included test should
be included to
>> > appropriate
>> > >> > > siutes.
>> > >> > > >> > > > > Creating IgniteLostAndFoundTestSuite,java
is not
>> > >> acceptable.
>> > >> > > >> > > > >
>> > >> > > >> > > > > 3) In case you're not sure what
to do with particular
>> > >> tests,
>> > >> > > >> please
>> > >> > > >> > > > provide
>> > >> > > >> > > > > lists of such tests. Please
group tests by "problem".
>> > >> > > >> > > > >
>> > >> > > >> > > > >
>> > >> > > >> > > > > On Fri, Feb 2, 2018 at 12:28
AM, Dmitry Pavlov <
>> > >> > > >> > dpavlov.spb@gmail.com>
>> > >> > > >> > > > > wrote:
>> > >> > > >> > > > >
>> > >> > > >> > > > > > Hi Ilya,
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > Thank you for this research.
I think it is useful
>> for
>> > >> > > community
>> > >> > > >> to
>> > >> > > >> > > > > identify
>> > >> > > >> > > > > > and remove obsolete tests
(if any), and include lost
>> > test
>> > >> > into
>> > >> > > >> CI
>> > >> > > >> > run
>> > >> > > >> > > > > chain
>> > >> > > >> > > > > > (if applicable).
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > For test with main() methods
I suggest to ask
>> authors
>> > >> (git
>> > >> > > >> > annotate)
>> > >> > > >> > > > and
>> > >> > > >> > > > > if
>> > >> > > >> > > > > > there is no response probably
we should remove such
>> > code.
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > Since I am not sure all
tests in this lost&found
>> suite
>> > >> are
>> > >> > > quite
>> > >> > > >> > > > stable I
>> > >> > > >> > > > > > suggest to create standalone
TC Run configuration
>> for
>> > >> such
>> > >> > > >> tests.
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > Earlier I've removed most
of tests causing timeouts
>> > from
>> > >> > basic
>> > >> > > >> > suite.
>> > >> > > >> > > > > > Ideally Basic suite should
contain fast run quite
>> > stable
>> > >> > > tests (
>> > >> > > >> > and
>> > >> > > >> > > 0
>> > >> > > >> > > > > > flaky ) because it is included
into RunAllBasic sub
>> set
>> > >> to
>> > >> > > brief
>> > >> > > >> > > commit
>> > >> > > >> > > > > > check  (
>> > >> > > >> > > > > > https://ci.ignite.apache.org/
>> > viewType.html?buildTypeId=
>> > >> > > >> > > > > IgniteTests24Java8_
>> > >> > > >> > > > > > RunBasicTests
>> > >> > > >> > > > > >  ).
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > Sincerely,
>> > >> > > >> > > > > > Dmitriy Pavlov
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > чт, 1 февр. 2018
г. в 20:22, Ilya Kasnacheev <
>> > >> > > >> > > > ilya.kasnacheev@gmail.com
>> > >> > > >> > > > > >:
>> > >> > > >> > > > > >
>> > >> > > >> > > > > > > Hello!
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > > While working on Ignite,
I have noticed that not
>> all
>> > >> tests
>> > >> > > >> are in
>> > >> > > >> > > any
>> > >> > > >> > > > > > test
>> > >> > > >> > > > > > > suite, hence I expect
they are ignored. I have
>> also
>> > >> > noticed
>> > >> > > >> some
>> > >> > > >> > > > files
>> > >> > > >> > > > > in
>> > >> > > >> > > > > > > src/test and named
*Test.java are actually
>> runnable
>> > >> > > >> main-classes
>> > >> > > >> > > and
>> > >> > > >> > > > > not
>> > >> > > >> > > > > > > tests. I think they're
ignored to. Also I've
>> noticed
>> > >> that
>> > >> > 6
>> > >> > > >> tests
>> > >> > > >> > > > > repeat
>> > >> > > >> > > > > > > twice.
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > > I have tried to fix
it by introducing "lost and
>> > found"
>> > >> > test
>> > >> > > >> > suite.
>> > >> > > >> > > > Not
>> > >> > > >> > > > > > sure
>> > >> > > >> > > > > > > what to do with main-classes.
I have also renamed
>> > >> abstract
>> > >> > > >> test
>> > >> > > >> > > > classes
>> > >> > > >> > > > > > to
>> > >> > > >> > > > > > > *AbstractTest.
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > > Please consider pull
request
>> > >> https://github.com/apache/
>> > >> > > >> > > > > ignite/pull/3464
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > > I have started this
suite on TC but I expect it to
>> > >> hang or
>> > >> > > >> worse.
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > >
>> > >> > https://ci.ignite.apache.org/viewLog.html?buildId=1071504&
>> > >> > > >> > > > > > tab=queuedBuildOverviewTab
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > > > Regards,
>> > >> > > >> > > > > > > --
>> > >> > > >> > > > > > > Ilya Kasnacheev
>> > >> > > >> > > > > > >
>> > >> > > >> > > > > >
>> > >> > > >> > > > >
>> > >> > > >> > > >
>> > >> > > >> > >
>> > >> > > >> >
>> > >> > > >>
>> > >> > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> > >
>> > >
>> >
>>
>

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