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 Tue, 22 May 2018 15:59:50 GMT
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