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 Wed, 01 Aug 2018 09:52:14 GMT
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