ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eduard Shangareev <eduard.shangar...@gmail.com>
Subject Re: Orphaned, duplicate, and main-class tests!
Date Fri, 25 May 2018 15:16:41 GMT
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