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: Orphaned, duplicate, and main-class tests!
Date Thu, 03 May 2018 17:03:20 GMT
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