You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@ignite.apache.org by Anton Vinogradov <av...@apache.org> on 2019/02/06 14:22:53 UTC

beforeTestsStarted() was deprecated, what should we use instead?

beforeTestsStarted() was deprecated recently.

See following comment at beforeTestsStarted():
* @deprecated This method is deprecated. Instead of invoking or overriding
it, it is recommended to make your own
* method with {@code @BeforeClass} annotation.

But, according to Junit documentation:
* Annotating a public static void no-arg method with @BeforeClass causes it
to be run once before any of the test methods in the class.

So, the method should be *static* to be annotated with @BeforeClass.
And I see a problem here since you're not able to use non-static methods
like GridAbstractTest#startGrid() at method annotated with @BeforeClass.

So, what is the real replacement?

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Anton Vinogradov <av...@apache.org>.

I vote for any proper solution :)

For now, we have to rollback the deprecation.

On Thu, Feb 7, 2019 at 11:46 AM Павлухин Иван <vo...@gmail.com> wrote:

> Hi,
>
> AFAIK various startGrid* methods of GridAbstractTest are not tied to
> concrete instance of test class. I think that a we can make them
> static. Also same can be done for some other methods and all they can
> be extracted into static utility class like GridTestSupport in order
> to reduce GridAbstractTest class size and separate concerns.
>
> What do you think?
>
> чт, 7 февр. 2019 г. в 09:44, oignatenko <oi...@gridgain.com>:
> >
> > Hi Anton,
> >
> > You can start and stop grid from static context required by mentioned
> > annotations using code like:
> >
> >      MyTestClass.class.newInstance().startGrid();
> >
> > This way doesn't look very convenient and I created JIRA task to address
> > that:
> >
> >     https://issues.apache.org/jira/browse/IGNITE-11240
> >
> > Does that address your question?
> >
> > regards, Oleg
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Павлухин Иван <vo...@gmail.com>.

Nikolay,

I agree with all points. But one thing is very uncertain. What
contribution is really troublesome (and consequently should be fixed
quickly or reverted)? But in fact it is a topic for a separate
discussion.

In current case about deprecation I agree with Nikolay and Anton. It
should be reverted until we have a substitution for beforeTestsStarted
method.

пн, 18 февр. 2019 г. в 13:01, Nikolay Izhikov <ni...@apache.org>:
>
> Ivan, I think we should use common sense here.
>
> 1. We should keep master consistent.
> 2. Contributors should try to fix issues appeared with their contribution.
>
> Do you agree with this?
>
> If some contributor can't fix issue quickly, is't completely OK.
> But, we should find another contributor, or revert contribution and rework
> it properly.
>
> There is no problem with some bad commit(I made such too).
> There is issues with bad commit fixing time.
>
>
> пн, 18 февр. 2019 г. в 12:34, Павлухин Иван <vo...@gmail.com>:
>
> > Nikolay,
> >
> > I agree with you and Anton that discussed deprecation is not a good thing.
> >
> > But it is quite interesting for me personally where our community
> > declared actions which should be taken against troublesome
> > contributions.
> >
> > пн, 18 февр. 2019 г. в 12:11, Nikolay Izhikov <ni...@apache.org>:
> > >
> > > Ivan.
> > >
> > > Please, use the quote right: "We should avoid any discussions *this
> > time*."
> > > This means - "We should *fix the issue* as quick as we can and discuss
> > > correct improvement after"
> > >
> > > Should we live this long with wrong deprecation?
> > > I don't think so.
> > >
> > >
> > > пн, 18 февр. 2019 г. в 11:58, Павлухин Иван <vo...@gmail.com>:
> > >
> > > > Anton,
> > > >
> > > > Could you please provide a link to a section in Ignite development
> > > > process document (or alike) stating in what cases "we should avoid a
> > > > discussion" and proceed with steps suggested by you? Sincerely, I am
> > > > not aware. Sorry if I missed it.
> > > >
> > > > пн, 18 февр. 2019 г. в 11:33, Anton Vinogradov <av...@apache.org>:
> > > > >
> > > > > Oleg,
> > > > >
> > > > > Task creation is not equaled to task in progress.
> > > > > It means nothing, to be honest.
> > > > > Are you going to fix this issue?
> > > > >
> > > > > That's not the first time we have such discussion at the community.
> > > > > So, now I have a clear vision.
> > > > > Since you contributed code with problems you are responsible to
> > > > > - fix them or
> > > > > - find the person who able to fix or
> > > > > - the code will be rolled back.
> > > > >
> > > > > We should avoid any discussions this time.
> > > > > Please tell me what case should we chose.
> > > > >
> > > > > On Sun, Feb 17, 2019 at 10:14 AM oignatenko <oignatenko@gridgain.com
> > >
> > > > wrote:
> > > > >
> > > > > > Hi Anton,
> > > > > >
> > > > > > I would like to re-check if I correctly understand the plan.
> > > > > >
> > > > > > So, we are going to remove deprecation from these two methods -
> > and the
> > > > > > reason for this is that the way recommended instead of these
> > methods
> > > > makes
> > > > > > it less convenient to start and stop the grid.
> > > > > >
> > > > > > Now, regarding the mentioned inconvenient way, we also plan to
> > improve
> > > > it
> > > > > > (JIRA task IGNITE-11240 if memory serves). My understanding is,
> > after
> > > > we
> > > > > > are
> > > > > > done with it, we will recover deprecation for these methods
> > because as
> > > > > > follows from above, inconvenience of using the recommended way was
> > the
> > > > only
> > > > > > reason to remove it.
> > > > > >
> > > > > > Please feel free to correct if I missed something.
> > > > > >
> > > > > > regards, Oleg
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Ivan Pavlukhin
> > > >
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >



-- 
Best regards,
Ivan Pavlukhin

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Nikolay Izhikov <ni...@apache.org>.

Ivan, I think we should use common sense here.

1. We should keep master consistent.
2. Contributors should try to fix issues appeared with their contribution.

Do you agree with this?

If some contributor can't fix issue quickly, is't completely OK.
But, we should find another contributor, or revert contribution and rework
it properly.

There is no problem with some bad commit(I made such too).
There is issues with bad commit fixing time.


пн, 18 февр. 2019 г. в 12:34, Павлухин Иван <vo...@gmail.com>:

> Nikolay,
>
> I agree with you and Anton that discussed deprecation is not a good thing.
>
> But it is quite interesting for me personally where our community
> declared actions which should be taken against troublesome
> contributions.
>
> пн, 18 февр. 2019 г. в 12:11, Nikolay Izhikov <ni...@apache.org>:
> >
> > Ivan.
> >
> > Please, use the quote right: "We should avoid any discussions *this
> time*."
> > This means - "We should *fix the issue* as quick as we can and discuss
> > correct improvement after"
> >
> > Should we live this long with wrong deprecation?
> > I don't think so.
> >
> >
> > пн, 18 февр. 2019 г. в 11:58, Павлухин Иван <vo...@gmail.com>:
> >
> > > Anton,
> > >
> > > Could you please provide a link to a section in Ignite development
> > > process document (or alike) stating in what cases "we should avoid a
> > > discussion" and proceed with steps suggested by you? Sincerely, I am
> > > not aware. Sorry if I missed it.
> > >
> > > пн, 18 февр. 2019 г. в 11:33, Anton Vinogradov <av...@apache.org>:
> > > >
> > > > Oleg,
> > > >
> > > > Task creation is not equaled to task in progress.
> > > > It means nothing, to be honest.
> > > > Are you going to fix this issue?
> > > >
> > > > That's not the first time we have such discussion at the community.
> > > > So, now I have a clear vision.
> > > > Since you contributed code with problems you are responsible to
> > > > - fix them or
> > > > - find the person who able to fix or
> > > > - the code will be rolled back.
> > > >
> > > > We should avoid any discussions this time.
> > > > Please tell me what case should we chose.
> > > >
> > > > On Sun, Feb 17, 2019 at 10:14 AM oignatenko <oignatenko@gridgain.com
> >
> > > wrote:
> > > >
> > > > > Hi Anton,
> > > > >
> > > > > I would like to re-check if I correctly understand the plan.
> > > > >
> > > > > So, we are going to remove deprecation from these two methods -
> and the
> > > > > reason for this is that the way recommended instead of these
> methods
> > > makes
> > > > > it less convenient to start and stop the grid.
> > > > >
> > > > > Now, regarding the mentioned inconvenient way, we also plan to
> improve
> > > it
> > > > > (JIRA task IGNITE-11240 if memory serves). My understanding is,
> after
> > > we
> > > > > are
> > > > > done with it, we will recover deprecation for these methods
> because as
> > > > > follows from above, inconvenience of using the recommended way was
> the
> > > only
> > > > > reason to remove it.
> > > > >
> > > > > Please feel free to correct if I missed something.
> > > > >
> > > > > regards, Oleg
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Павлухин Иван <vo...@gmail.com>.

Nikolay,

I agree with you and Anton that discussed deprecation is not a good thing.

But it is quite interesting for me personally where our community
declared actions which should be taken against troublesome
contributions.

пн, 18 февр. 2019 г. в 12:11, Nikolay Izhikov <ni...@apache.org>:
>
> Ivan.
>
> Please, use the quote right: "We should avoid any discussions *this time*."
> This means - "We should *fix the issue* as quick as we can and discuss
> correct improvement after"
>
> Should we live this long with wrong deprecation?
> I don't think so.
>
>
> пн, 18 февр. 2019 г. в 11:58, Павлухин Иван <vo...@gmail.com>:
>
> > Anton,
> >
> > Could you please provide a link to a section in Ignite development
> > process document (or alike) stating in what cases "we should avoid a
> > discussion" and proceed with steps suggested by you? Sincerely, I am
> > not aware. Sorry if I missed it.
> >
> > пн, 18 февр. 2019 г. в 11:33, Anton Vinogradov <av...@apache.org>:
> > >
> > > Oleg,
> > >
> > > Task creation is not equaled to task in progress.
> > > It means nothing, to be honest.
> > > Are you going to fix this issue?
> > >
> > > That's not the first time we have such discussion at the community.
> > > So, now I have a clear vision.
> > > Since you contributed code with problems you are responsible to
> > > - fix them or
> > > - find the person who able to fix or
> > > - the code will be rolled back.
> > >
> > > We should avoid any discussions this time.
> > > Please tell me what case should we chose.
> > >
> > > On Sun, Feb 17, 2019 at 10:14 AM oignatenko <oi...@gridgain.com>
> > wrote:
> > >
> > > > Hi Anton,
> > > >
> > > > I would like to re-check if I correctly understand the plan.
> > > >
> > > > So, we are going to remove deprecation from these two methods - and the
> > > > reason for this is that the way recommended instead of these methods
> > makes
> > > > it less convenient to start and stop the grid.
> > > >
> > > > Now, regarding the mentioned inconvenient way, we also plan to improve
> > it
> > > > (JIRA task IGNITE-11240 if memory serves). My understanding is, after
> > we
> > > > are
> > > > done with it, we will recover deprecation for these methods because as
> > > > follows from above, inconvenience of using the recommended way was the
> > only
> > > > reason to remove it.
> > > >
> > > > Please feel free to correct if I missed something.
> > > >
> > > > regards, Oleg
> > > >
> > > >
> > > >
> > > > --
> > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > >
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >



-- 
Best regards,
Ivan Pavlukhin

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Nikolay Izhikov <ni...@apache.org>.

Ivan.

Please, use the quote right: "We should avoid any discussions *this time*."
This means - "We should *fix the issue* as quick as we can and discuss
correct improvement after"

Should we live this long with wrong deprecation?
I don't think so.


пн, 18 февр. 2019 г. в 11:58, Павлухин Иван <vo...@gmail.com>:

> Anton,
>
> Could you please provide a link to a section in Ignite development
> process document (or alike) stating in what cases "we should avoid a
> discussion" and proceed with steps suggested by you? Sincerely, I am
> not aware. Sorry if I missed it.
>
> пн, 18 февр. 2019 г. в 11:33, Anton Vinogradov <av...@apache.org>:
> >
> > Oleg,
> >
> > Task creation is not equaled to task in progress.
> > It means nothing, to be honest.
> > Are you going to fix this issue?
> >
> > That's not the first time we have such discussion at the community.
> > So, now I have a clear vision.
> > Since you contributed code with problems you are responsible to
> > - fix them or
> > - find the person who able to fix or
> > - the code will be rolled back.
> >
> > We should avoid any discussions this time.
> > Please tell me what case should we chose.
> >
> > On Sun, Feb 17, 2019 at 10:14 AM oignatenko <oi...@gridgain.com>
> wrote:
> >
> > > Hi Anton,
> > >
> > > I would like to re-check if I correctly understand the plan.
> > >
> > > So, we are going to remove deprecation from these two methods - and the
> > > reason for this is that the way recommended instead of these methods
> makes
> > > it less convenient to start and stop the grid.
> > >
> > > Now, regarding the mentioned inconvenient way, we also plan to improve
> it
> > > (JIRA task IGNITE-11240 if memory serves). My understanding is, after
> we
> > > are
> > > done with it, we will recover deprecation for these methods because as
> > > follows from above, inconvenience of using the recommended way was the
> only
> > > reason to remove it.
> > >
> > > Please feel free to correct if I missed something.
> > >
> > > regards, Oleg
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Павлухин Иван <vo...@gmail.com>.

Anton,

Could you please provide a link to a section in Ignite development
process document (or alike) stating in what cases "we should avoid a
discussion" and proceed with steps suggested by you? Sincerely, I am
not aware. Sorry if I missed it.

пн, 18 февр. 2019 г. в 11:33, Anton Vinogradov <av...@apache.org>:
>
> Oleg,
>
> Task creation is not equaled to task in progress.
> It means nothing, to be honest.
> Are you going to fix this issue?
>
> That's not the first time we have such discussion at the community.
> So, now I have a clear vision.
> Since you contributed code with problems you are responsible to
> - fix them or
> - find the person who able to fix or
> - the code will be rolled back.
>
> We should avoid any discussions this time.
> Please tell me what case should we chose.
>
> On Sun, Feb 17, 2019 at 10:14 AM oignatenko <oi...@gridgain.com> wrote:
>
> > Hi Anton,
> >
> > I would like to re-check if I correctly understand the plan.
> >
> > So, we are going to remove deprecation from these two methods - and the
> > reason for this is that the way recommended instead of these methods makes
> > it less convenient to start and stop the grid.
> >
> > Now, regarding the mentioned inconvenient way, we also plan to improve it
> > (JIRA task IGNITE-11240 if memory serves). My understanding is, after we
> > are
> > done with it, we will recover deprecation for these methods because as
> > follows from above, inconvenience of using the recommended way was the only
> > reason to remove it.
> >
> > Please feel free to correct if I missed something.
> >
> > regards, Oleg
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >



-- 
Best regards,
Ivan Pavlukhin

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by oignatenko <ol...@googlemail.com>.

Hi Anton,

I re-checked details of the JIRA task you mentioned [1] and it looks like
finished to me, sorry. Ticket description is a list of specific items to be
addressed and as far as I can tell all of them are done.

Since you mentioned review, changes made per this ticket were fairly
thoroughly reviewed: for example respective PR [2] lists five commits that
were done to address review comments. And as far as I remember at the time
of review we had a solid consensus that scope of these changes is determined
by concrete items listed in task description.

Note by the way that after merge of these changes it became impossible to
write JUnit3 tests inheriting GridAbstractTest. One can probably call this
part of the changes final.

---

Speaking of JUnit3TestLegacySupport, TODOs are there to help readers see
that some specific parts of code in this class are expected to change per
particular JIRA task. That it should be (eventually) completely removed or
somehow reworked in the future is irrelevant here because this can be done
gradually, in different time, for different reasons and by different JIRA
tasks.

---

With regards to what you call "final Junit3 removal", I find it hard to
comprehend what exactly you want nor how this can be expressed in practical
actions of changing concrete code.

In contrast, the way how migration effort was explained at dev list
discussion that started it looks clear and understandable: "Is it time to
start translating tests to Unit 4 style?" (worth noting that it further goes
into even more concrete and practical explanation of what change is desired
and why, explaining that we want to be able to use @Ignore annotation to
simplify maintenance at Teamcity) And umbrella JIRA task that followed this
discussion [4] also has actionable description of what is to be done: "This
task is to migrate all tests to single uniform version Junit 4".

Speaking in terms of mentioned discussion and JIRA task, this is all done
and over - finished and verified ([5]). In fact, anyone can verify whether
migration is completed by doing plain text search for "junit.framework" in
project files and reviewing search results to make sure that there are no
test classes / suites inheriting the old API (JUnit 3 package names start
with junit.framework). It's very easy, just give it a try.

Summing up, I don't know what you have in mind speaking of what else should
be done or finalized, but whatever it is, I can't see how it would fit into
the scope of our original discussion here and of original JIRA task.

[1] https://issues.apache.org/jira/browse/IGNITE-10177
[2] https://github.com/apache/ignite/pull/5615
[3]
http://apache-ignite-developers.2346864.n4.nabble.com/Is-it-time-to-move-forward-to-JUnit4-5-td29608.html
[4] https://issues.apache.org/jira/browse/IGNITE-10173
[5] https://issues.apache.org/jira/browse/IGNITE-10629

regards, Oleg

Anton Vinogradov-2 wrote
> Oleg,
> 
> You may use TODO if it is really necessary.
> I see no reason to have any todo at JUnit3TestLegacySupport class since
> it's already marked as legacy and should be removed in the future.
> And we should have an issue about this removal.
> 
> Why you closed GNITE-10177 "cleanup Junit 3 from the project" since you're
> not finished work?
> Why now we discuss incorrect deprecation instead of making proper fix
> design in advance?
> 
> One more question to the committer.
> Dmitriy,
> 
> Why incomplete fix was merged to master without any issues responsible for
> finalization?
> Who's now responsible for final Junit3 removal?
> 
> On Mon, Feb 18, 2019 at 4:22 PM oignatenko &lt;

> oignatenko@

> &gt; wrote:
> 
>> Hi Anton,
>>
>> I removed @deprecated from javadoc, thanks for noticing. PR #6122 [1] is
>> updated.
>>
>> As for TODOs, I think you are wrong here because Ignite coding guidelines
>> [2] not only explicitly allow these but also explain the format in which
>> these should be presented. If you find violations of recommended TODOs
>> format, please let me know.
>>
>> regards, Oleg
>>
>> [1]: https://github.com/apache/ignite/pull/6122
>> [2]:
>>
>> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-TODOs
>>
>>
>>
>> --
>> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>>

--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: beforeTestsStarted() was deprecated, what should we use instead?

Posted by Dmitriy Pavlov <dp...@apache.org>.

Anton,

Do you have real questions? Seems like you're trying to state your opinion
related to the fix rather than asking a question.

We've discussed it in the thread related to Failure Handler fix, that whole
community is responsible to make things better. Mostly, contributors, who
care (proof of it is contribution). You can't say directly to someone to be
responsible (there is no boss in open source). If Oleg wants to proceed
with fixing, he will.

Please merge PR 6122 <https://github.com/apache/ignite/pull/6122> if you
agree with the new approach. We sometimes use todos with linking to tickets.

Sincerely,
Dmitriy Pavlov

пн, 18 февр. 2019 г. в 17:09, Anton Vinogradov <av...@apache.org>:

> Oleg,
>
> You may use TODO if it is really necessary.
> I see no reason to have any todo at JUnit3TestLegacySupport class since
> it's already marked as legacy and should be removed in the future.
> And we should have an issue about this removal.
>
> Why you closed GNITE-10177 "cleanup Junit 3 from the project" since you're
> not finished work?
> Why now we discuss incorrect deprecation instead of making proper fix
> design in advance?
>
> One more question to the committer.
> Dmitriy,
>
> Why incomplete fix was merged to master without any issues responsible for
> finalization?
> Who's now responsible for final Junit3 removal?
>
> On Mon, Feb 18, 2019 at 4:22 PM oignatenko <oi...@gridgain.com>
> wrote:
>
> > Hi Anton,
> >
> > I removed @deprecated from javadoc, thanks for noticing. PR #6122 [1] is
> > updated.
> >
> > As for TODOs, I think you are wrong here because Ignite coding guidelines
> > [2] not only explicitly allow these but also explain the format in which
> > these should be presented. If you find violations of recommended TODOs
> > format, please let me know.
> >
> > regards, Oleg
> >
> > [1]: https://github.com/apache/ignite/pull/6122
> > [2]:
> >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-TODOs
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>