ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Павлухин Иван <vololo...@gmail.com>
Subject Re: Migration to JUnit 5
Date Fri, 05 Jul 2019 08:32:42 GMT
Ivan,

I think that it is a really good that you found those not tested
examples. Thank you!

пт, 5 июл. 2019 г. в 11:31, Павлухин Иван <vololo100@gmail.com>:
>
> Ivan,
>
> I uncommented all tests referring to IGNITE-711 [1] in
> BasicExamplesSelfTest and all they passed.
>
> Generally, example tests are needed to be sure that our examples
> launch. And commented tests refer to existing examples. So, an ideal
> way here is to uncomment them in scope of IGNITE-711 [1], removal is
> not a good option. And I do not expect much problems here because we
> fully support Java 8 for a long time.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-711
>
> пн, 1 июл. 2019 г. в 17:10, Ivan Fedotov <ivanan639@gmail.com>:
> >
> > Hi, Igniters
> >
> > During work on IEP-30, which is about JUnit migration, I found that some
> > tests in examples module were commented [1] with the remark, that they
> > should be fixed in the ticket IGNITE-711 [2] which is about the
> > implementation of Java 8 examples.
> >
> > In the context of the ticket IGNITE-10973 [3] I want to uncomment them and
> > mark as @Disabled. Is it really need to disable mentioned tests or I can
> > just remove them as outdated?
> >
> > [1]
> > https://github.com/apache/ignite/pull/6606/files#diff-ed48193d25d777a2c30c187fa20a1a65L65
> > [2] https://issues.apache.org/jira/browse/IGNITE-711
> > [3] https://issues.apache.org/jira/browse/IGNITE-10973
> >
> >
> > вт, 26 февр. 2019 г. в 18:51, Ivan Fedotov <ivanan639@gmail.com>:
> >
> > > Ivan,
> > > I will investigate GridAbstractTest refactoring issue more precisely when
> > > I finish with JUnit3Legacy classes. Anyway, I will keep in touch with you
> > > and the community on the most significant moments.
> > >
> > > JUnit5 docs say that functionality is not full "especially with regard to
> > > reporting". On the other hand, I also agree with docs that it is the
> > > easiest way that does not require to touch CI infrastructure. I am going to
> > > try @RunWith(JUnitPlatform.class) construction with features from IEP to
> > > make sure that we will have the full support of them. The alternative way
> > > is dynamic tests [1], but the problem is that we add methods to suites
> > > manually, not via @Test annotation. It is some kind of rollback to JUnit3
> > > syntax.
> > >
> > > Anton,
> > > thank you for the reminder, I will update IEP according to the
> > > conversation.
> > >
> > > [1] https://www.baeldung.com/junit5-dynamic-tests
> > >
> > > вт, 26 февр. 2019 г. в 17:56, Anton Vinogradov <av@apache.org>:
> > >
> > >> Folks,
> > >>
> > >> Please make sure you keep IEP updated and each issue mentioned.
> > >>
> > >> On Tue, Feb 26, 2019 at 4:28 PM Павлухин Иван <vololo100@gmail.com>
> > >> wrote:
> > >>
> > >> > Ivan,
> > >> >
> > >> > Thank you for detailed answers! I would put a great care to
> > >> > @RunWith(JUnitPlatform.class) construction. As stated in junit5 docs
> > >> > [1] it does not support all features and unfortunately it is not clear
> > >> > how limited it is. Also, it is some kind of transitional mechanism
> > >> > which was not designed for being a long term solution.
> > >> >
> > >> > And I fully support an idea of refactoring GridAbstractTest. I think
> > >> > it is possible to make a significant improvement here.
> > >> >
> > >> > [1]
> > >> >
> > >> https://junit.org/junit5/docs/current/user-guide/#running-tests-junit-platform-runner
> > >> >
> > >> > пн, 25 февр. 2019 г. в 17:41, Ivan Fedotov <ivanan639@gmail.com>:
> > >> > >
> > >> > > Hello Nikolay.
> > >> > >
> > >> > > The prime benefits are more comfortable work with flaky tests,
Java 8
> > >> > tests
> > >> > > compatibility, user-friendly syntaxis in parametrized tests and
> > >> others.
> > >> > > The most significant features list you can find in IEP-30 Motivation
> > >> > > section.
> > >> > >
> > >> > > If you have any specific questions about JUnit5 feel free to
ask me.
> > >> > >
> > >> > > пн, 25 февр. 2019 г. в 16:55, Nikolay Izhikov <nizhikov@apache.org>:
> > >> > >
> > >> > > > Hello, Ivan.
> > >> > > >
> > >> > > > May be I miss some mail - if yes, can you repeat it.
> > >> > > > What is advantages of migration from junit 4 to 5?
> > >> > > > Why we should do it?
> > >> > > >
> > >> > > >
> > >> > > > пн, 25 февр. 2019 г. в 16:33, Ivan Fedotov <ivanan639@gmail.com>:
> > >> > > >
> > >> > > > > Ivan,
> > >> > > > > That is my thoughts according to your questions.
> > >> > > > >
> > >> > > > > 1. I tried to implement test suits with JUnit4 compatibility
> > >> layer.
> > >> > The
> > >> > > > > basic concept is to use @RunWith(JUnitPlatform.class)
> > >> @SelectClasses
> > >> > > > > ({...})[1] and on
> > >> > > > > CI Ignite it works fine.
> > >> > > > >
> > >> > > > > 2. According to @Rules, there are several ways to solve
it:
> > >> > > > >     2.1 Leave JUnit4 code without changes. It will
work because of
> > >> > > > Vintage
> > >> > > > > module
> > >> > > > >     2.2 Rewrite the @Rule as an Extension. The work
of extension
> > >> is
> > >> > > > similar
> > >> > > > > to the @Rules work, but it is extracted in an Extension
class.
> > >> > > > >     For more information about extensions, please,
follow the IEP
> > >> > [2].
> > >> > > > > In my opinion, the easiest and the most understandable
way is to
> > >> > leave
> > >> > > > > GridAbstractTest in current form. It will work with
JUnit5
> > >> > > > > syntaxis and abilities.
> > >> > > > >
> > >> > > > > 3. I faced a couple of problems during dealing with
dynamic and
> > >> > static
> > >> > > > > tests in one project with JUnit5. The problem occurs
with surefire
> > >> > > > version:
> > >> > > > > static tests work fine with 2.21x and earlier and with
dynamic
> > >> > tests, the
> > >> > > > > situation is vice versa, it works with > 2.21x surefire
version.
> > >> > > > > We can use helpful surefire dependency to use static
tests with
> > >> the
> > >> > > > newest
> > >> > > > > surefire version [3], but dynamic tests become unavailable
from
> > >> pure
> > >> > > > > Maven and accordingly from CI Ignite (from IDE all
is fine).
> > >> > > > > I can suggest leaving this type of tests on JUnit4
on the current
> > >> > stage -
> > >> > > > > they are in the vast minority.
> > >> > > > >
> > >> > > > > Let me comment on your side notes.
> > >> > > > >
> > >> > > > > I am not against the stable and widely-used test library
usage.
> > >> All I
> > >> > > > want
> > >> > > > > to say that it is not necessary in case of the main
testing Ignite
> > >> > > > > framework (Junit) already provides the mentioned features.
> > >> > > > >
> > >> > > > > At the initial stage of improvements 3->4 I am planning
to remove
> > >> > > > > JUnit3TestLegacyAssert, JUnit3TestLegacySupport classes.
I guess
> > >> that
> > >> > > > > during this work
> > >> > > > > I will face with an issue that you are mentioned -
turning
> > >> instance
> > >> > > > methods
> > >> > > > > to static. It is because of beforeTestsStarted and
> > >> afterTestsStarted
> > >> > > > > methods - I want to replace them by methods with BeforeAll,
> > >> AfterAll
> > >> > > > > annotations. But the point is that methods under such
annotations
> > >> > must be
> > >> > > > > static. Just now I am not sure about fully removing
> > >> > > > > GridCommonAbstractTest class, but the need for static
methods is a
> > >> > fact.
> > >> > > > >
> > >> > > > > [1]
> > >> > > > >
> > >> > > > >
> > >> > > >
> > >> >
> > >> https://github.com/apache/ignite/blob/85ba3a88d661bb05bbb749bd1feaf60cd9099ddc/examples/src/test/java/org/apache/ignite/testsuites/IgniteExamplesSelfTestSuite.java#L59
> > >> > > > > [2]
> > >> > > > >
> > >> > > > >
> > >> > > >
> > >> >
> > >> https://cwiki.apache.org/confluence/display/IGNITE/IEP-30%3A+Migration+to+JUnit+5
> > >> > > > > [3] https://github.com/junit-team/junit5/issues/1778
> > >> > > > >
> > >> > > > > вс, 24 февр. 2019 г. в 10:15, Павлухин
Иван <vololo100@gmail.com
> > >> >:
> > >> > > > >
> > >> > > > > > Ivan,
> > >> > > > > >
> > >> > > > > > Indeed junit5 has a lot of powerful features which
can improve
> > >> > testing
> > >> > > > > > process.
> > >> > > > > >
> > >> > > > > > But first we should go through a migration process.
There are
> > >> > several
> > >> > > > > > items which looks quite challenging.
> > >> > > > > > 1. Test suites support. Correct me if I am missed
it, but I have
> > >> > not
> > >> > > > > > found a concept of test suites similar to junit3/4
ones. CI in
> > >> > Ignite
> > >> > > > > > heavily depends on test suites. Is there an alternative
in
> > >> junit5?
> > >> > > > > > 2. The majority of our tests extend GridAbstractTest
which in
> > >> fact
> > >> > is
> > >> > > > > > a core class in Ignite testing. Writing a test
without extending
> > >> > it is
> > >> > > > > > not a good idea. Currently it employs number of
junit4 concepts,
> > >> > e.g.
> > >> > > > > > test rules which as I saw are not supported in
junit5. So, it
> > >> > sounds
> > >> > > > > > that some changes in GridAbstractTest need to
be done. During
> > >> > > > > > migration from junit 3 to 4 GridAbstractTest used
kind of
> > >> mimicry,
> > >> > it
> > >> > > > > > can be used as a base class for junit3 and junit4
tests at the
> > >> same
> > >> > > > > > time. How can we address transitional period now?
> > >> > > > > > 3. Also we have bunch of tests using our home-brewed
> > >> > parametrization.
> > >> > > > > > You can find them by searching usages of
> > >> > > > > > ConfigVariationsTestSuiteBuilder. This part was
rather tricky
> > >> > during
> > >> > > > > > migration to junit4.
> > >> > > > > >
> > >> > > > > > Do we have a plan for all these items?
> > >> > > > > > ----
> > >> > > > > >
> > >> > > > > > Couple of side notes.
> > >> > > > > >
> > >> > > > > > Regarding dependencies minimization. Actually,
I think it is
> > >> > important
> > >> > > > > > for junit itself as a library. Many libraries
try to minimize
> > >> > > > > > dependency. In Ignite we do so as well. But in
my opinion it is
> > >> not
> > >> > > > > > the case in context of libraries used during testing.
If we have
> > >> > > > > > useful, stable and widely-used test library which
can improve
> > >> our
> > >> > > > > > processes why should not we use it?
> > >> > > > > >
> > >> > > > > > Regarding removing leftovers left after junit
3->4 migration.
> > >> > > > > > Actually, I think that GridAbstractTest and
> > >> GridCommonAbstractTest
> > >> > can
> > >> > > > > > be refactored in order to simplify further development
and
> > >> > migration
> > >> > > > > > to new testing framework. For example, there are
a lot of
> > >> instance
> > >> > > > > > methods which can be turned to static methods.
Various
> > >> > start/stopGrid
> > >> > > > > > methods fall into this category. They can be extracted
into some
> > >> > > > > > utility class and imported statically. Perhaps,
after number of
> > >> > such
> > >> > > > > > refactoring we will be able to write tests without
extending
> > >> > > > > > GridCommonAbstractTest.
> > >> > > > > >
> > >> > > > > > пт, 22 февр. 2019 г. в 18:33, Ivan Fedotov
<ivanan639@gmail.com
> > >> >:
> > >> > > > > > >
> > >> > > > > > > Hi Ivan!
> > >> > > > > > >
> > >> > > > > > >  Junit5 differs from JUnit4 not so strong
as 4 from 3
> > >> version. Of
> > >> > > > > course,
> > >> > > > > > > we can use AssertJ and other libraries, but
it is more
> > >> > comfortable to
> > >> > > > > > > use functionality from the box. Moreover,
the JUnit team
> > >> provides
> > >> > > > > strong
> > >> > > > > > > support for its products and it is the core
JUnit principle -
> > >> > > > minimize
> > >> > > > > > > third-party dependency [1].
> > >> > > > > > >
> > >> > > > > > >  According to Parameterized tests, it has
several cons in
> > >> JUnit4:
> > >> > > > > > >  1. Test classes use fields and constructors
to define
> > >> > parameters,
> > >> > > > > which
> > >> > > > > > > make tests more verbose
> > >> > > > > > >  2. It requires a separate test class for
each method being
> > >> > tested.
> > >> > > > > > >  In JUnit5 it has a simplified parameter
syntax and supports
> > >> > multiple
> > >> > > > > > > data-set source types, including CSV and
annotation
> > >> > > > > > >
> > >> > > > > > >  Impact on daily test development does not
so differ from
> > >> > development
> > >> > > > > on
> > >> > > > > > > JUnit4. We also can use annotations to mark
methods as tests,
> > >> but
> > >> > > > some
> > >> > > > > > main
> > >> > > > > > > annotations have
> > >> > > > > > > different names - you can see it in the ticket
description
> > >> [2].
> > >> > You
> > >> > > > > have
> > >> > > > > > to
> > >> > > > > > > use those annotations and different import,
but these are
> > >> minor
> > >> > > > > changes.
> > >> > > > > > >  We can change suites from static to dynamic
tests [3], but I
> > >> am
> > >> > not
> > >> > > > > sure
> > >> > > > > > > that it is necessary. If you have any arguments
in favor of
> > >> > dynamic
> > >> > > > > > tests,
> > >> > > > > > > I am ready to discuss them.
> > >> > > > > > >
> > >> > > > > > >  Now I see that changes in GridAbstractTest
are not required.
> > >> > Only
> > >> > > > > > > improvements in JUnit 3->4 migration,
which were given in IEP.
> > >> > Other
> > >> > > > > > JUnit5
> > >> > > > > > > features we can use with additional imports.
The problem can
> > >> > appear
> > >> > > > > with
> > >> > > > > > > dynamic tests because we can not launch static
and dynamic
> > >> under
> > >> > one
> > >> > > > > > > surefire version. I made a preliminary migration
on examples
> > >> > module,
> > >> > > > > you
> > >> > > > > > > can take a look on it [4], but now it is
still in work.
> > >> > > > > > >
> > >> > > > > > > I tried to find some other JUnit5 features
and added them to
> > >> > IEP. If
> > >> > > > I
> > >> > > > > > miss
> > >> > > > > > > something, please, let me now, we will also
take it into
> > >> account.
> > >> > > > > > >
> > >> > > > > > >  [1]
> > >> https://github.com/junit-team/junit5/wiki/Core-Principles
> > >> > > > > > >  [2] https://issues.apache.org/jira/browse/IGNITE-10958
> > >> > > > > > >  [3] https://www.baeldung.com/junit5-dynamic-tests
> > >> > > > > > >  [4] https://github.com/apache/ignite/pull/5888
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > чт, 21 февр. 2019 г. в 18:45, Павлухин
Иван <
> > >> vololo100@gmail.com
> > >> > >:
> > >> > > > > > >
> > >> > > > > > > > Hi Ivan,
> > >> > > > > > > >
> > >> > > > > > > > Thank you for your efforts!
> > >> > > > > > > >
> > >> > > > > > > > I checked a section "Motivation" in
IEP and I think that we
> > >> > should
> > >> > > > > add
> > >> > > > > > > > more details there. You provided mostly
examples of more
> > >> > convenient
> > >> > > > > > > > assertions. But there are other options
to deal with it.
> > >> E.g.
> > >> > > > AssertJ
> > >> > > > > > > > library [1] (I think that we can consider
it even after
> > >> > migration
> > >> > > > to
> > >> > > > > > > > junit5). It would be great if we can
describe some junit5
> > >> > features
> > >> > > > > > > > which can make our life simpler and
there is no alternative
> > >> in
> > >> > > > > junit4.
> > >> > > > > > > > E.g. we have the similar Parameterized
concept in junit4,
> > >> so it
> > >> > > > does
> > >> > > > > > > > not look as a big win here.
> > >> > > > > > > >
> > >> > > > > > > > Also, an impact on everyday development
should be estimated.
> > >> > As I
> > >> > > > > > > > know, junit5 has a compatibility layer
which allows to
> > >> migrate
> > >> > from
> > >> > > > > > > > junit4 seamlessly. But as I understood
you would like to use
> > >> > new
> > >> > > > > > > > junit5 features. And we have well-known
GridAbstractTest
> > >> which
> > >> > > > > > > > historically was bound to junit3, now
is bound to junit4.
> > >> > Should we
> > >> > > > > > > > change it significantly for junit5?
Should we change other
> > >> > existing
> > >> > > > > > > > tests? Suites?
> > >> > > > > > > >
> > >> > > > > > > > Could you please address my concerns?
> > >> > > > > > > >
> > >> > > > > > > > Let's discuss pros and cons. I will
be happy to help there.
> > >> > > > > > > >
> > >> > > > > > > > [1] http://joel-costigliola.github.io/assertj/
> > >> > > > > > > >
> > >> > > > > > > > чт, 21 февр. 2019 г. в 18:07,
Ivan Fedotov <
> > >> > ivanan639@gmail.com>:
> > >> > > > > > > > >
> > >> > > > > > > > > Dmitriy, thank you, access is fine.
> > >> > > > > > > > >
> > >> > > > > > > > > I have created the corresponding
IEP [1].
> > >> > > > > > > > >
> > >> > > > > > > > > Now I am going to continue work
on this. If somebody has
> > >> any
> > >> > > > > > suggestions
> > >> > > > > > > > or
> > >> > > > > > > > > additions I am ready to discuss
them.
> > >> > > > > > > > >
> > >> > > > > > > > > [1]
> > >> > > > > > > > >
> > >> > > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> >
> > >> https://cwiki.apache.org/confluence/display/IGNITE/IEP-30%3A+Migration+to+JUnit+5
> > >> > > > > > > > >
> > >> > > > > > > > > чт, 21 февр. 2019 г. в
01:42, Dmitriy Pavlov <
> > >> > dpavlov@apache.org
> > >> > > > >:
> > >> > > > > > > > >
> > >> > > > > > > > > > Done, please check access
now.
> > >> > > > > > > > > >
> > >> > > > > > > > > > ср, 20 февр. 2019 г.
в 21:49, Ivan Fedotov <
> > >> > > > ivanan639@gmail.com
> > >> > > > > >:
> > >> > > > > > > > > >
> > >> > > > > > > > > > > Dmitriy, thank you for
the response.
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > My wiki username is "ivanan",
the related mailbox is
> > >> > > > > > > > ivanan639@gmail.com
> > >> > > > > > > > > > .
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > ср, 20 февр. 2019
г. в 18:38, Dmitriy Pavlov <
> > >> > > > > dpavlov@apache.org
> > >> > > > > > >:
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > > Hi Ivan,
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Now admin service
is unavailable (gives error 503).
> > >> > I'll
> > >> > > > add
> > >> > > > > > rights
> > >> > > > > > > > > > once
> > >> > > > > > > > > > > it
> > >> > > > > > > > > > > > is up and running.
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Could you share
your wiki username? I can't find any
> > >> > users
> > >> > > > > who
> > >> > > > > > > > signed
> > >> > > > > > > > > > up
> > >> > > > > > > > > > > in
> > >> > > > > > > > > > > > the wiki with any
similar email/username
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Sincerely,
> > >> > > > > > > > > > > > Dmitriy Pavlov
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > ср, 20 февр.
2019 г. в 18:26, Ivan Fedotov <
> > >> > > > > > ivanan639@gmail.com>:
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > > Hi, Igniters.
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > I am planning
to formalize migration to JUnit5 and
> > >> > create
> > >> > > > > IEP
> > >> > > > > > > > which
> > >> > > > > > > > > > > will
> > >> > > > > > > > > > > > > include related
issues.
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > I already started
to work on one of the issues [1]
> > >> > and
> > >> > > > > > created a
> > >> > > > > > > > > > draft
> > >> > > > > > > > > > > > for
> > >> > > > > > > > > > > > > the corresponding
IEP [2].
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > Please, give
me rights for confluence to create
> > >> IEP.
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > [1]
> > >> > https://issues.apache.org/jira/browse/IGNITE-10973
> > >> > > > > > > > > > > > > [2]
> > >> > > > > > > >
> > >> > https://gist.github.com/1vanan/1f81319f1dc6d6ebca30c216fdd82759
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > Sincerely,
> > >> > > > > > > > > > > > > Ivan Fedotov.
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > > > ivanan639@gmail.com
> > >> > > > > > > > > > > > >
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > --
> > >> > > > > > > > > > > Ivan Fedotov.
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > ivanan639@gmail.com
> > >> > > > > > > > > > >
> > >> > > > > > > > > >
> > >> > > > > > > > >
> > >> > > > > > > > >
> > >> > > > > > > > > --
> > >> > > > > > > > > Ivan Fedotov.
> > >> > > > > > > > >
> > >> > > > > > > > > ivanan639@gmail.com
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > --
> > >> > > > > > > > Best regards,
> > >> > > > > > > > Ivan Pavlukhin
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > --
> > >> > > > > > > Ivan Fedotov.
> > >> > > > > > >
> > >> > > > > > > ivanan639@gmail.com
> > >> > > > > >
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > --
> > >> > > > > > Best regards,
> > >> > > > > > Ivan Pavlukhin
> > >> > > > > >
> > >> > > > >
> > >> > > > >
> > >> > > > > --
> > >> > > > > Ivan Fedotov.
> > >> > > > >
> > >> > > > > ivanan639@gmail.com
> > >> > > > >
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > > Ivan Fedotov.
> > >> > >
> > >> > > ivanan639@gmail.com
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Best regards,
> > >> > Ivan Pavlukhin
> > >> >
> > >>
> > >
> > >
> > > --
> > > Ivan Fedotov.
> > >
> > > ivanan639@gmail.com
> > >
> >
> >
> > --
> > Ivan Fedotov.
> >
> > ivanan639@gmail.com
>
>
>
> --
> Best regards,
> Ivan Pavlukhin



-- 
Best regards,
Ivan Pavlukhin

Mime
View raw message