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 Wed, 31 Oct 2018 12:22:15 GMT
Hello!

So we have uncommented 4 batches out of 10! 6 to go. Some broken
functionality were exposed.

There is still work to do, so do not hesitate to assign a subtask to
yourself.

Regards,
-- 
Ilya Kasnacheev


ср, 15 авг. 2018 г. в 19:42, Ilya Kasnacheev <ilya.kasnacheev@gmail.com>:

> Hello!
>
> So we have enabled a first batch of tests:
> https://github.com/apache/ignite/pull/4504
>
> How it was done: I have uncommented classes. Some of these were absent in
> code base, so I have checked if we didn't lose anything important - they
> were testing CLOCK mode which isn't with us for some time, so I removed
> their entries.
> Then I have ran them, some were broken. Most of those were testing on-heap
> caching with copy=false, which now requires setOnheapCaching(true), which I
> did. After that, cache.invoke() still didn't work, so I commented this part
> out.
> The remaining test was broken due to dependence on hash map iteration
> order, which was changed in Java 8. So I have got the remaining tests
> working, checking important parts of our system.
>
> Please do not hesitate to assign subtasks of
> https://issues.apache.org/jira/browse/IGNITE-9210 to yourself, dabble
> with tests. IMO it's the best way for a novice developer to become
> acquainted with Ignite code base, tests and history, while helping the
> project.
>
> Thanks,
>
> --
> Ilya Kasnacheev
>
> 2018-08-07 16:54 GMT+03:00 Ilya Kasnacheev <ilya.kasnacheev@gmail.com>:
>
>> Hello!
>>
>> Thank you Dmitriy, and thanks to everybody who participated in
>> discussions.
>>
>> I have created tickets for next steps:
>> https://issues.apache.org/jira/browse/IGNITE-9210 (with subtasks)
>> https://issues.apache.org/jira/browse/IGNITE-9222
>> https://issues.apache.org/jira/browse/IGNITE-9223
>>
>> As usual, feedback will be very welcome.
>>
>> Regards,
>>
>> --
>> Ilya Kasnacheev
>>
>> 2018-08-07 13:58 GMT+03:00 Dmitriy Pavlov <dpavlov.spb@gmail.com>:
>>
>>> 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