You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Christian Schulte <cs...@schulte.it> on 2017/01/09 23:21:21 UTC
[IT MNG-5958]: Please review integration test for MNG-5958
Commit to review is here:
<https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca683bfc0c9373>
If no one objects until Friday, 13th, 2017, I'll merge it to master.
Regards,
--
Christian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Christian Schulte <cs...@schulte.it>.
Am 01/16/17 um 00:06 schrieb Anton Tanasenko:
> I've submitted the PR for MNG-5958 branch.
> Changed the existing 5805 IT to work for 3.3.9 only ('phases' syntax), and
> duplicated it to a one that works with 3.3.9+ ('lifecyclePhases' syntax).
Thanks. I just committed this to the MNG-5958 branch. Should I start a
second thread for review?
Regards,
--
Christian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Anton Tanasenko <at...@gmail.com>.
I've submitted the PR for MNG-5958 branch.
Changed the existing 5805 IT to work for 3.3.9 only ('phases' syntax), and
duplicated it to a one that works with 3.3.9+ ('lifecyclePhases' syntax).
2017-01-11 22:59 GMT+02:00 Christian Schulte <cs...@schulte.it>:
> Am 01/11/17 um 15:05 schrieb Anton Tanasenko:
> > I'll submit a PR in these couple of days, if it waits a little bit.
>
> No hurry here.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards,
Anton.
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Christian Schulte <cs...@schulte.it>.
Am 01/11/17 um 15:05 schrieb Anton Tanasenko:
> I'll submit a PR in these couple of days, if it waits a little bit.
No hurry here.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Anton Tanasenko <at...@gmail.com>.
I'll submit a PR in these couple of days, if it waits a little bit.
2017-01-11 0:54 GMT+02:00 Christian Schulte <cs...@schulte.it>:
> Am 01/10/17 um 09:30 schrieb Anton Tanasenko:
> > 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle
> > goals as
> > '<lifecyclePhases><..><mojos><mojo><goal/><configuration/><
> dependencies/></mojo</mojos</...></lifecyclePhases>'
> > in addition to '<phases><...>[goals as text]</...></phases>', but due to
> > implementation, it was also supported using the 'phases' parent node and
> > the test was using that one as well.
> > This broke binary compatibility, which is fixed in 5958, but as a
> > side-effect, 'phases' node can no longer be used for the new syntax.
> >
> > If strictly following the rules that you brought up, the test for 5805
> > should really be duplicated and changed to use the new syntax starting
> > 3.5.0, but to be honest, I'm not sure if it's really worth it.
>
> Adding/updating ITs without review has led to ITs testing
> incorrect/unexpected behaviour which is hard to change. Having the old
> test unchanged with an upper bound and a new test in paralle testing the
> expected/new/correct behaviour helps to track what behaviour got
> invalidated by what release. It's not that much extra work since you can
> copy the old test over to a new class and just need to change some bits
> to make it fit. Please either create a pull request or attach a patch in
> JIRA. When it comes to the core IT repository, things cannot be changed
> without discussion. In fact, this is what required to reset various
> branches. I'll add a link to this thread in the commit message so that
> everyone kicking in months later can read about why things are the way
> the are.
>
> Regards,
> --
> Christian
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards,
Anton.
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Christian Schulte <cs...@schulte.it>.
Am 01/10/17 um 09:30 schrieb Anton Tanasenko:
> 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle
> goals as
> '<lifecyclePhases><..><mojos><mojo><goal/><configuration/><dependencies/></mojo</mojos</...></lifecyclePhases>'
> in addition to '<phases><...>[goals as text]</...></phases>', but due to
> implementation, it was also supported using the 'phases' parent node and
> the test was using that one as well.
> This broke binary compatibility, which is fixed in 5958, but as a
> side-effect, 'phases' node can no longer be used for the new syntax.
>
> If strictly following the rules that you brought up, the test for 5805
> should really be duplicated and changed to use the new syntax starting
> 3.5.0, but to be honest, I'm not sure if it's really worth it.
Adding/updating ITs without review has led to ITs testing
incorrect/unexpected behaviour which is hard to change. Having the old
test unchanged with an upper bound and a new test in paralle testing the
expected/new/correct behaviour helps to track what behaviour got
invalidated by what release. It's not that much extra work since you can
copy the old test over to a new class and just need to change some bits
to make it fit. Please either create a pull request or attach a patch in
JIRA. When it comes to the core IT repository, things cannot be changed
without discussion. In fact, this is what required to reset various
branches. I'll add a link to this thread in the commit message so that
everyone kicking in months later can read about why things are the way
the are.
Regards,
--
Christian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Robert Scholte <rf...@apache.org>.
I remember MNG-5805 and noticed the change of the signature. I expected
this code to be Maven-internal only, so it looked fine to me. Now it seems
it is not.
We had the same issue when improving toolchains, which required a
signature change as well (being able to merge global with user
toolchains). The assumption was that maven-toolchains-plugin was the only
plugin using this specific code, so we adjusted it to make it compatible
with both signatures. Again there was another thirdparty maven-plugin
hitting the same issue.
I think we should accept there are 2 signatures in the world now. On the
plugin side there's always extra handling to be done, either refuse
certain Maven versions or cope with both signatures.
Since the damage is already done and because the change gives you more
info instead of less, I would advice to keep the code as-is and to provide
a little code-example to solve this with reflection.
And yes, this still means we need to create ITs for both situations.
Robert
On Tue, 10 Jan 2017 10:48:32 +0100, Stephen Connolly
<st...@gmail.com> wrote:
> It sounds to me that the intent for 3.3.9 was that it would work with
> <phases> and the test confirmed it as working that way.
>
> As such the impression I get is that this is neither a false positive
> nor a
> false negative test. There should have been a <lifecyclePhases> test for
> 3.3.9, and we are intentionally changing the behaviour with <phases> so
> that it no longer supports the new syntax because support for the new
> syntax broke backwards compatibility.
>
> So my vote is that we mark the existing test as range=[3.3.9,3.5.0) and
> add
> a new test (which is a clone of this but switched to <lifecyclePhases>
> that
> is active for the range=[3.3.9,)
>
> WDYT?
>
> (Requesting second opinion from Jason, Hervê & Robert)
>
> -Stephen
>
> On Tue 10 Jan 2017 at 08:30, Anton Tanasenko <at...@gmail.com>
> wrote:
>
>> 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle
>>
>> goals as
>>
>>
>> '<lifecyclePhases><..><mojos><mojo><goal/><configuration/><dependencies/></mojo</mojos</...></lifecyclePhases>'
>>
>> in addition to '<phases><...>[goals as text]</...></phases>', but due to
>>
>> implementation, it was also supported using the 'phases' parent node and
>>
>> the test was using that one as well.
>>
>> This broke binary compatibility, which is fixed in 5958, but as a
>>
>> side-effect, 'phases' node can no longer be used for the new syntax.
>>
>>
>>
>> If strictly following the rules that you brought up, the test for 5805
>>
>> should really be duplicated and changed to use the new syntax starting
>>
>> 3.5.0, but to be honest, I'm not sure if it's really worth it.
>>
>>
>>
>> For one, this feature was not announced in any meaningful way, I used it
>>
>> for a POC at my old job (in fact using 'lifecyclePhases'), which will
>> never
>>
>> be used, and I've also suggested it in a different issue (also with the
>>
>> 'lifecyclePhases') to a user.
>>
>> So for the sake of the test representing the feature, if anyone comes
>>
>> across it while looking for a way to use it, I'd also leave only one
>>
>> instance of the test to prevent the confusion.
>>
>>
>>
>> And for two, there are multiple dirs that are used by the test under
>>
>> core-it-support and core-it-suite, most of which would need to be
>>
>> duplicated. For a feature so seldomly used, duplicating would do more
>> harm
>>
>> than good.
>>
>>
>>
>>
>>
>> Given the above, I'd say the test was in error in the first place, it
>>
>> should have been written with 'lifecyclePhases' from start.
>>
>>
>>
>>
>>
>>
>>
>> 2017-01-10 9:51 GMT+02:00 Stephen Connolly <
>> stephen.alan.connolly@gmail.com>
>>
>> :
>>
>>
>>
>> > I'll rephrase.
>>
>> >
>>
>> > That test is currently passing on 3.3.9. Why?
>>
>> >
>>
>> > If that testing passing on 3.3.9 because 3.3.9 was (badly) designed to
>> work
>>
>> > that way? If yes then the test stays, change the range to
>> [3.3.9,3.5.0)
>> and
>>
>> > duplicate the test with the duplicate having the change and the range
>> being
>>
>> > [3.5.0,)
>>
>> >
>>
>> > If that test is passing on 3.3.9 because the test doesn't actually
>> test
>>
>> > what it was intended to test, then it is fine to change that test
>>
>> >
>>
>> > That the new version of the test will pass on 3.3.9 doesn't because of
>> some
>>
>> > side effect is not a valid reason to change an existing test.
>>
>> >
>>
>> > The test was run against 3.3.9 and 3.3.9 was released with that test
>>
>> > passing, unless you are absolutely certain that the test is a
>>
>> > false-positive or false-negative, we do not change the test (other
>> than
>>
>> > range activation) rather we duplicate the test (second one being _mk2
>> or
>>
>> > something like that) and ensure that the duplicate has a different
>> range
>>
>> > activation
>>
>> >
>>
>> > HTH
>>
>> >
>>
>> > On 10 January 2017 at 07:32, Stephen Connolly <
>>
>> > stephen.alan.connolly@gmail.com> wrote:
>>
>> >
>>
>> > > So 5805 should be marked as only for [3.3.9] and then copy it for
>> the
>>
>> > > rephrased version
>>
>> > >
>>
>> > > On Tue 10 Jan 2017 at 06:17, Anton Tanasenko
>> <at...@gmail.com>
>>
>> > > wrote:
>>
>> > >
>>
>> > >> Looks about right.
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >> Stephen, the change to MNG-5805 test as part of MNG-5958 was
>>
>> > intentional,
>>
>> > >>
>>
>> > >> since I broke binary compat in the initial implementation of the
>>
>> > feature.
>>
>> > >>
>>
>> > >> The changed test should also work with 3.3.9 which supported both
>>
>> > 'phases'
>>
>> > >>
>>
>> > >> and 'lifecyclePhases' for the extended config, while after MNG-5958
>> one
>>
>> > >>
>>
>> > >> should only use 'lifecyclePhases' for that.
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >> 2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >> > Hi,
>>
>> > >>
>>
>> > >> >
>>
>> > >>
>>
>> > >> > forgot to add those email addresses in the CC. Sending it again
>> with
>>
>> > the
>>
>> > >>
>>
>> > >> > authors in the CC.
>>
>> > >>
>>
>> > >> >
>>
>> > >>
>>
>> > >> >
>>
>> > >>
>>
>> > >> > Am 01/10/17 um 00:59 schrieb Christian Schulte:
>>
>> > >>
>>
>> > >> > > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
>>
>> > >>
>>
>> > >> > >> It seems you are modifying an existing test:
>>
>> > >>
>>
>> > >> > >> https://github.com/apache/maven-integration-testing/blob/
>>
>> > >>
>>
>> > >> > 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
>>
>> > >>
>>
>> > >> > core-it-plugins/mng5805-extension/src/main/resources/
>>
>> > >>
>>
>> > >> > META-INF/plexus/components.xml
>>
>> > >>
>>
>> > >> > >>
>>
>> > >>
>>
>> > >> > >> Integration tests should be append-only (with rare exceptions)
>>
>> > >>
>>
>> > >> > >>
>>
>> > >>
>>
>> > >> > >> If the resource needs to be changed then mark the test with an
>>
>> > upper
>>
>> > >>
>>
>> > >> > range
>>
>> > >>
>>
>> > >> > >> limit of ,3.5.0) and create the new test with a range limit of
>>
>> > >> [3.5.0,
>>
>> > >>
>>
>> > >> > >>
>>
>> > >>
>>
>> > >> > >> Changing existing tests is bad and was one of the reasons why
>> we
>>
>> > had
>>
>> > >> to
>>
>> > >>
>>
>> > >> > >> reset
>>
>> > >>
>>
>> > >> > >>
>>
>> > >>
>>
>> > >> > >> -1 on the current formulation of this change.
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > I am only the committer. I'll try to bring the authors of the
>>
>> > commits
>>
>> > >>
>>
>> > >> > > into the discussion.
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Commit in the core:
>>
>> > >>
>>
>> > >> > >
>> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > >>
>>
>> > >> > 71ada08978c78d3b1416f0cf4f63942dddb171d9>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Stuart McCulloch <mc...@gmail.com>
>>
>> > >>
>>
>> > >> > > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Commit to the ITs:
>>
>> > >>
>>
>> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > >>
>>
>> > >> > integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
>>
>> > >>
>>
>> > >> > 3bfc0c9373>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > >>
>>
>> > >> > > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Issue introducing the IT which needs to be changed:
>>
>> > >>
>>
>> > >> > > <https://issues.apache.org/jira/browse/MNG-5805>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Commits to the core:
>>
>> > >>
>>
>> > >> > >
>> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > >>
>>
>> > >> > 3677220f6e499e97f2b47b0593bc394b689d14d6>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > >>
>>
>> > >> > > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > >
>> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > >>
>>
>> > >> > 9f7971dadbec8882b4c119345494b620d3a1f897>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > >>
>>
>> > >> > > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Commits to the ITs due to this:
>>
>> > >>
>>
>> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > >>
>>
>> > >> > integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
>>
>> > >>
>>
>> > >> > f27c8360f1>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > >>
>>
>> > >> > > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > >>
>>
>> > >> > integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
>>
>> > >>
>>
>> > >> > 8e5cb1a11e>
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > >>
>>
>> > >> > > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > The author of the IT being changed appears to be the same
>> author
>> who
>>
>> > >>
>>
>> > >> > > contributed the initial version.
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > +1 from me (committer) for the change.
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> > > Regards,
>>
>> > >>
>>
>> > >> > >
>>
>> > >>
>>
>> > >> >
>>
>> > >>
>>
>> > >> >
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >>
>>
>> > >> --
>>
>> > >>
>>
>> > >> Regards,
>>
>> > >>
>>
>> > >> Anton.
>>
>> > >>
>>
>> > >> --
>>
>> > > Sent from my phone
>>
>> > >
>>
>> >
>>
>>
>>
>>
>>
>>
>>
>> --
>>
>> Regards,
>>
>> Anton.
>>
>> --
> Sent from my phone
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Stephen Connolly <st...@gmail.com>.
It sounds to me that the intent for 3.3.9 was that it would work with
<phases> and the test confirmed it as working that way.
As such the impression I get is that this is neither a false positive nor a
false negative test. There should have been a <lifecyclePhases> test for
3.3.9, and we are intentionally changing the behaviour with <phases> so
that it no longer supports the new syntax because support for the new
syntax broke backwards compatibility.
So my vote is that we mark the existing test as range=[3.3.9,3.5.0) and add
a new test (which is a clone of this but switched to <lifecyclePhases> that
is active for the range=[3.3.9,)
WDYT?
(Requesting second opinion from Jason, Hervê & Robert)
-Stephen
On Tue 10 Jan 2017 at 08:30, Anton Tanasenko <at...@gmail.com>
wrote:
> 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle
>
> goals as
>
>
> '<lifecyclePhases><..><mojos><mojo><goal/><configuration/><dependencies/></mojo</mojos</...></lifecyclePhases>'
>
> in addition to '<phases><...>[goals as text]</...></phases>', but due to
>
> implementation, it was also supported using the 'phases' parent node and
>
> the test was using that one as well.
>
> This broke binary compatibility, which is fixed in 5958, but as a
>
> side-effect, 'phases' node can no longer be used for the new syntax.
>
>
>
> If strictly following the rules that you brought up, the test for 5805
>
> should really be duplicated and changed to use the new syntax starting
>
> 3.5.0, but to be honest, I'm not sure if it's really worth it.
>
>
>
> For one, this feature was not announced in any meaningful way, I used it
>
> for a POC at my old job (in fact using 'lifecyclePhases'), which will never
>
> be used, and I've also suggested it in a different issue (also with the
>
> 'lifecyclePhases') to a user.
>
> So for the sake of the test representing the feature, if anyone comes
>
> across it while looking for a way to use it, I'd also leave only one
>
> instance of the test to prevent the confusion.
>
>
>
> And for two, there are multiple dirs that are used by the test under
>
> core-it-support and core-it-suite, most of which would need to be
>
> duplicated. For a feature so seldomly used, duplicating would do more harm
>
> than good.
>
>
>
>
>
> Given the above, I'd say the test was in error in the first place, it
>
> should have been written with 'lifecyclePhases' from start.
>
>
>
>
>
>
>
> 2017-01-10 9:51 GMT+02:00 Stephen Connolly <
> stephen.alan.connolly@gmail.com>
>
> :
>
>
>
> > I'll rephrase.
>
> >
>
> > That test is currently passing on 3.3.9. Why?
>
> >
>
> > If that testing passing on 3.3.9 because 3.3.9 was (badly) designed to
> work
>
> > that way? If yes then the test stays, change the range to [3.3.9,3.5.0)
> and
>
> > duplicate the test with the duplicate having the change and the range
> being
>
> > [3.5.0,)
>
> >
>
> > If that test is passing on 3.3.9 because the test doesn't actually test
>
> > what it was intended to test, then it is fine to change that test
>
> >
>
> > That the new version of the test will pass on 3.3.9 doesn't because of
> some
>
> > side effect is not a valid reason to change an existing test.
>
> >
>
> > The test was run against 3.3.9 and 3.3.9 was released with that test
>
> > passing, unless you are absolutely certain that the test is a
>
> > false-positive or false-negative, we do not change the test (other than
>
> > range activation) rather we duplicate the test (second one being _mk2 or
>
> > something like that) and ensure that the duplicate has a different range
>
> > activation
>
> >
>
> > HTH
>
> >
>
> > On 10 January 2017 at 07:32, Stephen Connolly <
>
> > stephen.alan.connolly@gmail.com> wrote:
>
> >
>
> > > So 5805 should be marked as only for [3.3.9] and then copy it for the
>
> > > rephrased version
>
> > >
>
> > > On Tue 10 Jan 2017 at 06:17, Anton Tanasenko <at...@gmail.com>
>
> > > wrote:
>
> > >
>
> > >> Looks about right.
>
> > >>
>
> > >>
>
> > >>
>
> > >> Stephen, the change to MNG-5805 test as part of MNG-5958 was
>
> > intentional,
>
> > >>
>
> > >> since I broke binary compat in the initial implementation of the
>
> > feature.
>
> > >>
>
> > >> The changed test should also work with 3.3.9 which supported both
>
> > 'phases'
>
> > >>
>
> > >> and 'lifecyclePhases' for the extended config, while after MNG-5958
> one
>
> > >>
>
> > >> should only use 'lifecyclePhases' for that.
>
> > >>
>
> > >>
>
> > >>
>
> > >> 2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
>
> > >>
>
> > >>
>
> > >>
>
> > >> > Hi,
>
> > >>
>
> > >> >
>
> > >>
>
> > >> > forgot to add those email addresses in the CC. Sending it again with
>
> > the
>
> > >>
>
> > >> > authors in the CC.
>
> > >>
>
> > >> >
>
> > >>
>
> > >> >
>
> > >>
>
> > >> > Am 01/10/17 um 00:59 schrieb Christian Schulte:
>
> > >>
>
> > >> > > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
>
> > >>
>
> > >> > >> It seems you are modifying an existing test:
>
> > >>
>
> > >> > >> https://github.com/apache/maven-integration-testing/blob/
>
> > >>
>
> > >> > 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
>
> > >>
>
> > >> > core-it-plugins/mng5805-extension/src/main/resources/
>
> > >>
>
> > >> > META-INF/plexus/components.xml
>
> > >>
>
> > >> > >>
>
> > >>
>
> > >> > >> Integration tests should be append-only (with rare exceptions)
>
> > >>
>
> > >> > >>
>
> > >>
>
> > >> > >> If the resource needs to be changed then mark the test with an
>
> > upper
>
> > >>
>
> > >> > range
>
> > >>
>
> > >> > >> limit of ,3.5.0) and create the new test with a range limit of
>
> > >> [3.5.0,
>
> > >>
>
> > >> > >>
>
> > >>
>
> > >> > >> Changing existing tests is bad and was one of the reasons why we
>
> > had
>
> > >> to
>
> > >>
>
> > >> > >> reset
>
> > >>
>
> > >> > >>
>
> > >>
>
> > >> > >> -1 on the current formulation of this change.
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > I am only the committer. I'll try to bring the authors of the
>
> > commits
>
> > >>
>
> > >> > > into the discussion.
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Commit in the core:
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > >>
>
> > >> > 71ada08978c78d3b1416f0cf4f63942dddb171d9>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Stuart McCulloch <mc...@gmail.com>
>
> > >>
>
> > >> > > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Commit to the ITs:
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > >>
>
> > >> > integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
>
> > >>
>
> > >> > 3bfc0c9373>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Anton Tanasenko <at...@gmail.com>
>
> > >>
>
> > >> > > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Issue introducing the IT which needs to be changed:
>
> > >>
>
> > >> > > <https://issues.apache.org/jira/browse/MNG-5805>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Commits to the core:
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > >>
>
> > >> > 3677220f6e499e97f2b47b0593bc394b689d14d6>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Anton Tanasenko <at...@gmail.com>
>
> > >>
>
> > >> > > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > >>
>
> > >> > 9f7971dadbec8882b4c119345494b620d3a1f897>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Anton Tanasenko <at...@gmail.com>
>
> > >>
>
> > >> > > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Commits to the ITs due to this:
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > >>
>
> > >> > integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
>
> > >>
>
> > >> > f27c8360f1>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Anton Tanasenko <at...@gmail.com>
>
> > >>
>
> > >> > > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > >>
>
> > >> > integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
>
> > >>
>
> > >> > 8e5cb1a11e>
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > author Anton Tanasenko <at...@gmail.com>
>
> > >>
>
> > >> > > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > The author of the IT being changed appears to be the same author
> who
>
> > >>
>
> > >> > > contributed the initial version.
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > +1 from me (committer) for the change.
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> > > Regards,
>
> > >>
>
> > >> > >
>
> > >>
>
> > >> >
>
> > >>
>
> > >> >
>
> > >>
>
> > >>
>
> > >>
>
> > >>
>
> > >>
>
> > >> --
>
> > >>
>
> > >> Regards,
>
> > >>
>
> > >> Anton.
>
> > >>
>
> > >> --
>
> > > Sent from my phone
>
> > >
>
> >
>
>
>
>
>
>
>
> --
>
> Regards,
>
> Anton.
>
> --
Sent from my phone
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Anton Tanasenko <at...@gmail.com>.
3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle
goals as
'<lifecyclePhases><..><mojos><mojo><goal/><configuration/><dependencies/></mojo</mojos</...></lifecyclePhases>'
in addition to '<phases><...>[goals as text]</...></phases>', but due to
implementation, it was also supported using the 'phases' parent node and
the test was using that one as well.
This broke binary compatibility, which is fixed in 5958, but as a
side-effect, 'phases' node can no longer be used for the new syntax.
If strictly following the rules that you brought up, the test for 5805
should really be duplicated and changed to use the new syntax starting
3.5.0, but to be honest, I'm not sure if it's really worth it.
For one, this feature was not announced in any meaningful way, I used it
for a POC at my old job (in fact using 'lifecyclePhases'), which will never
be used, and I've also suggested it in a different issue (also with the
'lifecyclePhases') to a user.
So for the sake of the test representing the feature, if anyone comes
across it while looking for a way to use it, I'd also leave only one
instance of the test to prevent the confusion.
And for two, there are multiple dirs that are used by the test under
core-it-support and core-it-suite, most of which would need to be
duplicated. For a feature so seldomly used, duplicating would do more harm
than good.
Given the above, I'd say the test was in error in the first place, it
should have been written with 'lifecyclePhases' from start.
2017-01-10 9:51 GMT+02:00 Stephen Connolly <st...@gmail.com>
:
> I'll rephrase.
>
> That test is currently passing on 3.3.9. Why?
>
> If that testing passing on 3.3.9 because 3.3.9 was (badly) designed to work
> that way? If yes then the test stays, change the range to [3.3.9,3.5.0) and
> duplicate the test with the duplicate having the change and the range being
> [3.5.0,)
>
> If that test is passing on 3.3.9 because the test doesn't actually test
> what it was intended to test, then it is fine to change that test
>
> That the new version of the test will pass on 3.3.9 doesn't because of some
> side effect is not a valid reason to change an existing test.
>
> The test was run against 3.3.9 and 3.3.9 was released with that test
> passing, unless you are absolutely certain that the test is a
> false-positive or false-negative, we do not change the test (other than
> range activation) rather we duplicate the test (second one being _mk2 or
> something like that) and ensure that the duplicate has a different range
> activation
>
> HTH
>
> On 10 January 2017 at 07:32, Stephen Connolly <
> stephen.alan.connolly@gmail.com> wrote:
>
> > So 5805 should be marked as only for [3.3.9] and then copy it for the
> > rephrased version
> >
> > On Tue 10 Jan 2017 at 06:17, Anton Tanasenko <at...@gmail.com>
> > wrote:
> >
> >> Looks about right.
> >>
> >>
> >>
> >> Stephen, the change to MNG-5805 test as part of MNG-5958 was
> intentional,
> >>
> >> since I broke binary compat in the initial implementation of the
> feature.
> >>
> >> The changed test should also work with 3.3.9 which supported both
> 'phases'
> >>
> >> and 'lifecyclePhases' for the extended config, while after MNG-5958 one
> >>
> >> should only use 'lifecyclePhases' for that.
> >>
> >>
> >>
> >> 2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
> >>
> >>
> >>
> >> > Hi,
> >>
> >> >
> >>
> >> > forgot to add those email addresses in the CC. Sending it again with
> the
> >>
> >> > authors in the CC.
> >>
> >> >
> >>
> >> >
> >>
> >> > Am 01/10/17 um 00:59 schrieb Christian Schulte:
> >>
> >> > > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
> >>
> >> > >> It seems you are modifying an existing test:
> >>
> >> > >> https://github.com/apache/maven-integration-testing/blob/
> >>
> >> > 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
> >>
> >> > core-it-plugins/mng5805-extension/src/main/resources/
> >>
> >> > META-INF/plexus/components.xml
> >>
> >> > >>
> >>
> >> > >> Integration tests should be append-only (with rare exceptions)
> >>
> >> > >>
> >>
> >> > >> If the resource needs to be changed then mark the test with an
> upper
> >>
> >> > range
> >>
> >> > >> limit of ,3.5.0) and create the new test with a range limit of
> >> [3.5.0,
> >>
> >> > >>
> >>
> >> > >> Changing existing tests is bad and was one of the reasons why we
> had
> >> to
> >>
> >> > >> reset
> >>
> >> > >>
> >>
> >> > >> -1 on the current formulation of this change.
> >>
> >> > >
> >>
> >> > > I am only the committer. I'll try to bring the authors of the
> commits
> >>
> >> > > into the discussion.
> >>
> >> > >
> >>
> >> > > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
> >>
> >> > >
> >>
> >> > > Commit in the core:
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> >>
> >> > 71ada08978c78d3b1416f0cf4f63942dddb171d9>
> >>
> >> > >
> >>
> >> > > author Stuart McCulloch <mc...@gmail.com>
> >>
> >> > > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
> >>
> >> > >
> >>
> >> > > Commit to the ITs:
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
> >>
> >> > integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
> >>
> >> > 3bfc0c9373>
> >>
> >> > >
> >>
> >> > > author Anton Tanasenko <at...@gmail.com>
> >>
> >> > > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
> >>
> >> > >
> >>
> >> > > Issue introducing the IT which needs to be changed:
> >>
> >> > > <https://issues.apache.org/jira/browse/MNG-5805>
> >>
> >> > >
> >>
> >> > > Commits to the core:
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> >>
> >> > 3677220f6e499e97f2b47b0593bc394b689d14d6>
> >>
> >> > >
> >>
> >> > > author Anton Tanasenko <at...@gmail.com>
> >>
> >> > > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
> >>
> >> > >
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> >>
> >> > 9f7971dadbec8882b4c119345494b620d3a1f897>
> >>
> >> > >
> >>
> >> > > author Anton Tanasenko <at...@gmail.com>
> >>
> >> > > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
> >>
> >> > >
> >>
> >> > > Commits to the ITs due to this:
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
> >>
> >> > integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
> >>
> >> > f27c8360f1>
> >>
> >> > >
> >>
> >> > > author Anton Tanasenko <at...@gmail.com>
> >>
> >> > > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
> >>
> >> > >
> >>
> >> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
> >>
> >> > integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
> >>
> >> > 8e5cb1a11e>
> >>
> >> > >
> >>
> >> > > author Anton Tanasenko <at...@gmail.com>
> >>
> >> > > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
> >>
> >> > >
> >>
> >> > > The author of the IT being changed appears to be the same author who
> >>
> >> > > contributed the initial version.
> >>
> >> > >
> >>
> >> > > +1 from me (committer) for the change.
> >>
> >> > >
> >>
> >> > > Regards,
> >>
> >> > >
> >>
> >> >
> >>
> >> >
> >>
> >>
> >>
> >>
> >>
> >> --
> >>
> >> Regards,
> >>
> >> Anton.
> >>
> >> --
> > Sent from my phone
> >
>
--
Regards,
Anton.
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Stephen Connolly <st...@gmail.com>.
I'll rephrase.
That test is currently passing on 3.3.9. Why?
If that testing passing on 3.3.9 because 3.3.9 was (badly) designed to work
that way? If yes then the test stays, change the range to [3.3.9,3.5.0) and
duplicate the test with the duplicate having the change and the range being
[3.5.0,)
If that test is passing on 3.3.9 because the test doesn't actually test
what it was intended to test, then it is fine to change that test
That the new version of the test will pass on 3.3.9 doesn't because of some
side effect is not a valid reason to change an existing test.
The test was run against 3.3.9 and 3.3.9 was released with that test
passing, unless you are absolutely certain that the test is a
false-positive or false-negative, we do not change the test (other than
range activation) rather we duplicate the test (second one being _mk2 or
something like that) and ensure that the duplicate has a different range
activation
HTH
On 10 January 2017 at 07:32, Stephen Connolly <
stephen.alan.connolly@gmail.com> wrote:
> So 5805 should be marked as only for [3.3.9] and then copy it for the
> rephrased version
>
> On Tue 10 Jan 2017 at 06:17, Anton Tanasenko <at...@gmail.com>
> wrote:
>
>> Looks about right.
>>
>>
>>
>> Stephen, the change to MNG-5805 test as part of MNG-5958 was intentional,
>>
>> since I broke binary compat in the initial implementation of the feature.
>>
>> The changed test should also work with 3.3.9 which supported both 'phases'
>>
>> and 'lifecyclePhases' for the extended config, while after MNG-5958 one
>>
>> should only use 'lifecyclePhases' for that.
>>
>>
>>
>> 2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
>>
>>
>>
>> > Hi,
>>
>> >
>>
>> > forgot to add those email addresses in the CC. Sending it again with the
>>
>> > authors in the CC.
>>
>> >
>>
>> >
>>
>> > Am 01/10/17 um 00:59 schrieb Christian Schulte:
>>
>> > > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
>>
>> > >> It seems you are modifying an existing test:
>>
>> > >> https://github.com/apache/maven-integration-testing/blob/
>>
>> > 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
>>
>> > core-it-plugins/mng5805-extension/src/main/resources/
>>
>> > META-INF/plexus/components.xml
>>
>> > >>
>>
>> > >> Integration tests should be append-only (with rare exceptions)
>>
>> > >>
>>
>> > >> If the resource needs to be changed then mark the test with an upper
>>
>> > range
>>
>> > >> limit of ,3.5.0) and create the new test with a range limit of
>> [3.5.0,
>>
>> > >>
>>
>> > >> Changing existing tests is bad and was one of the reasons why we had
>> to
>>
>> > >> reset
>>
>> > >>
>>
>> > >> -1 on the current formulation of this change.
>>
>> > >
>>
>> > > I am only the committer. I'll try to bring the authors of the commits
>>
>> > > into the discussion.
>>
>> > >
>>
>> > > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
>>
>> > >
>>
>> > > Commit in the core:
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > 71ada08978c78d3b1416f0cf4f63942dddb171d9>
>>
>> > >
>>
>> > > author Stuart McCulloch <mc...@gmail.com>
>>
>> > > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
>>
>> > >
>>
>> > > Commit to the ITs:
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
>>
>> > 3bfc0c9373>
>>
>> > >
>>
>> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
>>
>> > >
>>
>> > > Issue introducing the IT which needs to be changed:
>>
>> > > <https://issues.apache.org/jira/browse/MNG-5805>
>>
>> > >
>>
>> > > Commits to the core:
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > 3677220f6e499e97f2b47b0593bc394b689d14d6>
>>
>> > >
>>
>> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
>>
>> > >
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>>
>> > 9f7971dadbec8882b4c119345494b620d3a1f897>
>>
>> > >
>>
>> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
>>
>> > >
>>
>> > > Commits to the ITs due to this:
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
>>
>> > f27c8360f1>
>>
>> > >
>>
>> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
>>
>> > >
>>
>> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>>
>> > integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
>>
>> > 8e5cb1a11e>
>>
>> > >
>>
>> > > author Anton Tanasenko <at...@gmail.com>
>>
>> > > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
>>
>> > >
>>
>> > > The author of the IT being changed appears to be the same author who
>>
>> > > contributed the initial version.
>>
>> > >
>>
>> > > +1 from me (committer) for the change.
>>
>> > >
>>
>> > > Regards,
>>
>> > >
>>
>> >
>>
>> >
>>
>>
>>
>>
>>
>> --
>>
>> Regards,
>>
>> Anton.
>>
>> --
> Sent from my phone
>
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Stephen Connolly <st...@gmail.com>.
So 5805 should be marked as only for [3.3.9] and then copy it for the
rephrased version
On Tue 10 Jan 2017 at 06:17, Anton Tanasenko <at...@gmail.com>
wrote:
> Looks about right.
>
>
>
> Stephen, the change to MNG-5805 test as part of MNG-5958 was intentional,
>
> since I broke binary compat in the initial implementation of the feature.
>
> The changed test should also work with 3.3.9 which supported both 'phases'
>
> and 'lifecyclePhases' for the extended config, while after MNG-5958 one
>
> should only use 'lifecyclePhases' for that.
>
>
>
> 2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
>
>
>
> > Hi,
>
> >
>
> > forgot to add those email addresses in the CC. Sending it again with the
>
> > authors in the CC.
>
> >
>
> >
>
> > Am 01/10/17 um 00:59 schrieb Christian Schulte:
>
> > > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
>
> > >> It seems you are modifying an existing test:
>
> > >> https://github.com/apache/maven-integration-testing/blob/
>
> > 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
>
> > core-it-plugins/mng5805-extension/src/main/resources/
>
> > META-INF/plexus/components.xml
>
> > >>
>
> > >> Integration tests should be append-only (with rare exceptions)
>
> > >>
>
> > >> If the resource needs to be changed then mark the test with an upper
>
> > range
>
> > >> limit of ,3.5.0) and create the new test with a range limit of [3.5.0,
>
> > >>
>
> > >> Changing existing tests is bad and was one of the reasons why we had
> to
>
> > >> reset
>
> > >>
>
> > >> -1 on the current formulation of this change.
>
> > >
>
> > > I am only the committer. I'll try to bring the authors of the commits
>
> > > into the discussion.
>
> > >
>
> > > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
>
> > >
>
> > > Commit in the core:
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > 71ada08978c78d3b1416f0cf4f63942dddb171d9>
>
> > >
>
> > > author Stuart McCulloch <mc...@gmail.com>
>
> > > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
>
> > >
>
> > > Commit to the ITs:
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
>
> > 3bfc0c9373>
>
> > >
>
> > > author Anton Tanasenko <at...@gmail.com>
>
> > > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
>
> > >
>
> > > Issue introducing the IT which needs to be changed:
>
> > > <https://issues.apache.org/jira/browse/MNG-5805>
>
> > >
>
> > > Commits to the core:
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > 3677220f6e499e97f2b47b0593bc394b689d14d6>
>
> > >
>
> > > author Anton Tanasenko <at...@gmail.com>
>
> > > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
>
> > >
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
>
> > 9f7971dadbec8882b4c119345494b620d3a1f897>
>
> > >
>
> > > author Anton Tanasenko <at...@gmail.com>
>
> > > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
>
> > >
>
> > > Commits to the ITs due to this:
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
>
> > f27c8360f1>
>
> > >
>
> > > author Anton Tanasenko <at...@gmail.com>
>
> > > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
>
> > >
>
> > > <https://git-wip-us.apache.org/repos/asf?p=maven-
>
> > integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
>
> > 8e5cb1a11e>
>
> > >
>
> > > author Anton Tanasenko <at...@gmail.com>
>
> > > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
>
> > >
>
> > > The author of the IT being changed appears to be the same author who
>
> > > contributed the initial version.
>
> > >
>
> > > +1 from me (committer) for the change.
>
> > >
>
> > > Regards,
>
> > >
>
> >
>
> >
>
>
>
>
>
> --
>
> Regards,
>
> Anton.
>
> --
Sent from my phone
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Anton Tanasenko <at...@gmail.com>.
Looks about right.
Stephen, the change to MNG-5805 test as part of MNG-5958 was intentional,
since I broke binary compat in the initial implementation of the feature.
The changed test should also work with 3.3.9 which supported both 'phases'
and 'lifecyclePhases' for the extended config, while after MNG-5958 one
should only use 'lifecyclePhases' for that.
2017-01-10 6:13 GMT+02:00 Christian Schulte <cs...@schulte.it>:
> Hi,
>
> forgot to add those email addresses in the CC. Sending it again with the
> authors in the CC.
>
>
> Am 01/10/17 um 00:59 schrieb Christian Schulte:
> > Am 01/10/17 um 00:40 schrieb Stephen Connolly:
> >> It seems you are modifying an existing test:
> >> https://github.com/apache/maven-integration-testing/blob/
> 8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/
> core-it-plugins/mng5805-extension/src/main/resources/
> META-INF/plexus/components.xml
> >>
> >> Integration tests should be append-only (with rare exceptions)
> >>
> >> If the resource needs to be changed then mark the test with an upper
> range
> >> limit of ,3.5.0) and create the new test with a range limit of [3.5.0,
> >>
> >> Changing existing tests is bad and was one of the reasons why we had to
> >> reset
> >>
> >> -1 on the current formulation of this change.
> >
> > I am only the committer. I'll try to bring the authors of the commits
> > into the discussion.
> >
> > Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
> >
> > Commit in the core:
> > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> 71ada08978c78d3b1416f0cf4f63942dddb171d9>
> >
> > author Stuart McCulloch <mc...@gmail.com>
> > Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
> >
> > Commit to the ITs:
> > <https://git-wip-us.apache.org/repos/asf?p=maven-
> integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca68
> 3bfc0c9373>
> >
> > author Anton Tanasenko <at...@gmail.com>
> > Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
> >
> > Issue introducing the IT which needs to be changed:
> > <https://issues.apache.org/jira/browse/MNG-5805>
> >
> > Commits to the core:
> > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> 3677220f6e499e97f2b47b0593bc394b689d14d6>
> >
> > author Anton Tanasenko <at...@gmail.com>
> > Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
> >
> > <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=
> 9f7971dadbec8882b4c119345494b620d3a1f897>
> >
> > author Anton Tanasenko <at...@gmail.com>
> > Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
> >
> > Commits to the ITs due to this:
> > <https://git-wip-us.apache.org/repos/asf?p=maven-
> integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91ba
> f27c8360f1>
> >
> > author Anton Tanasenko <at...@gmail.com>
> > Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
> >
> > <https://git-wip-us.apache.org/repos/asf?p=maven-
> integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b80858
> 8e5cb1a11e>
> >
> > author Anton Tanasenko <at...@gmail.com>
> > Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
> >
> > The author of the IT being changed appears to be the same author who
> > contributed the initial version.
> >
> > +1 from me (committer) for the change.
> >
> > Regards,
> >
>
>
--
Regards,
Anton.
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Christian Schulte <cs...@schulte.it>.
Hi,
forgot to add those email addresses in the CC. Sending it again with the
authors in the CC.
Am 01/10/17 um 00:59 schrieb Christian Schulte:
> Am 01/10/17 um 00:40 schrieb Stephen Connolly:
>> It seems you are modifying an existing test:
>> https://github.com/apache/maven-integration-testing/blob/8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/core-it-plugins/mng5805-extension/src/main/resources/META-INF/plexus/components.xml
>>
>> Integration tests should be append-only (with rare exceptions)
>>
>> If the resource needs to be changed then mark the test with an upper range
>> limit of ,3.5.0) and create the new test with a range limit of [3.5.0,
>>
>> Changing existing tests is bad and was one of the reasons why we had to
>> reset
>>
>> -1 on the current formulation of this change.
>
> I am only the committer. I'll try to bring the authors of the commits
> into the discussion.
>
> Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
>
> Commit in the core:
> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=71ada08978c78d3b1416f0cf4f63942dddb171d9>
>
> author Stuart McCulloch <mc...@gmail.com>
> Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
>
> Commit to the ITs:
> <https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca683bfc0c9373>
>
> author Anton Tanasenko <at...@gmail.com>
> Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
>
> Issue introducing the IT which needs to be changed:
> <https://issues.apache.org/jira/browse/MNG-5805>
>
> Commits to the core:
> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=3677220f6e499e97f2b47b0593bc394b689d14d6>
>
> author Anton Tanasenko <at...@gmail.com>
> Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
>
> <https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=9f7971dadbec8882b4c119345494b620d3a1f897>
>
> author Anton Tanasenko <at...@gmail.com>
> Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
>
> Commits to the ITs due to this:
> <https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91baf27c8360f1>
>
> author Anton Tanasenko <at...@gmail.com>
> Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
>
> <https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b808588e5cb1a11e>
>
> author Anton Tanasenko <at...@gmail.com>
> Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
>
> The author of the IT being changed appears to be the same author who
> contributed the initial version.
>
> +1 from me (committer) for the change.
>
> Regards,
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Christian Schulte <cs...@schulte.it>.
Am 01/10/17 um 00:40 schrieb Stephen Connolly:
> It seems you are modifying an existing test:
> https://github.com/apache/maven-integration-testing/blob/8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/core-it-plugins/mng5805-extension/src/main/resources/META-INF/plexus/components.xml
>
> Integration tests should be append-only (with rare exceptions)
>
> If the resource needs to be changed then mark the test with an upper range
> limit of ,3.5.0) and create the new test with a range limit of [3.5.0,
>
> Changing existing tests is bad and was one of the reasons why we had to
> reset
>
> -1 on the current formulation of this change.
I am only the committer. I'll try to bring the authors of the commits
into the discussion.
Issue: <https://issues.apache.org/jira/browse/MNG-5958>.
Commit in the core:
<https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=71ada08978c78d3b1416f0cf4f63942dddb171d9>
author Stuart McCulloch <mc...@gmail.com>
Wed, 6 Jan 2016 12:23:06 +0100 (11:23 +0000)
Commit to the ITs:
<https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca683bfc0c9373>
author Anton Tanasenko <at...@gmail.com>
Thu, 7 Jan 2016 03:01:28 +0100 (04:01 +0200)
Issue introducing the IT which needs to be changed:
<https://issues.apache.org/jira/browse/MNG-5805>
Commits to the core:
<https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=3677220f6e499e97f2b47b0593bc394b689d14d6>
author Anton Tanasenko <at...@gmail.com>
Sun, 19 Jul 2015 22:01:50 +0100 (00:01 +0300)
<https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=9f7971dadbec8882b4c119345494b620d3a1f897>
author Anton Tanasenko <at...@gmail.com>
Sat, 1 Aug 2015 15:02:52 +0100 (17:02 +0300)
Commits to the ITs due to this:
<https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=63656ffd5cd9c5287715336a5f91baf27c8360f1>
author Anton Tanasenko <at...@gmail.com>
Sun, 19 Apr 2015 22:50:37 +0100 (00:50 +0300)
<https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=ef7b0d378305ab62aa7b3824b808588e5cb1a11e>
author Anton Tanasenko <at...@gmail.com>
Mon, 27 Apr 2015 22:58:32 +0100 (00:58 +0300)
The author of the IT being changed appears to be the same author who
contributed the initial version.
+1 from me (committer) for the change.
Regards,
--
Christian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: [IT MNG-5958]: Please review integration test for MNG-5958
Posted by Stephen Connolly <st...@gmail.com>.
It seems you are modifying an existing test:
https://github.com/apache/maven-integration-testing/blob/8852538208e508fdc7b58d6332ca683bfc0c9373/core-it-support/core-it-plugins/mng5805-extension/src/main/resources/META-INF/plexus/components.xml
Integration tests should be append-only (with rare exceptions)
If the resource needs to be changed then mark the test with an upper range
limit of ,3.5.0) and create the new test with a range limit of [3.5.0,
Changing existing tests is bad and was one of the reasons why we had to
reset
-1 on the current formulation of this change.
On Mon 9 Jan 2017 at 23:21, Christian Schulte <cs...@schulte.it> wrote:
> Commit to review is here:
>
>
>
> <
> https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=8852538208e508fdc7b58d6332ca683bfc0c9373
> >
>
>
>
> If no one objects until Friday, 13th, 2017, I'll merge it to master.
>
>
>
> Regards,
>
> --
>
> Christian
>
>
>
> ---------------------------------------------------------------------
>
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
>
> --
Sent from my phone