Re: [IT MNG-5958]: Please review integration test for MNG-5958

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.

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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 : > Am 01/11/17 um 15:05 schrieb Anton

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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-unsubscr...@maven.apache.org For additional commands, e-mail:

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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 : > Am 01/10/17 um 09:30 schrieb Anton Tanasenko: > > 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle > > goals as > > '<..>< >

Re: [IT MNG-5958]: Please review integration test for MNG-5958

Am 01/10/17 um 09:30 schrieb Anton Tanasenko: > 3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle > goals as > '<..>' > in addition to '<...>[goals as text]', but due to > implementation, it was also supported using the 'phases' parent node and > the test was using that one

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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

Re: [IT MNG-5958]: Please review integration test for MNG-5958

It sounds to me that the intent for 3.3.9 was that it would work with 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 test for 3.3.9, and we are intentionally changing the

Re: [IT MNG-5958]: Please review integration test for MNG-5958

3.3.9 (in 5805) introduced an additional syntax for specifying lifecycle goals as '<..>' in addition to '<...>[goals as text]', 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

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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 wrote: > Looks about right. > > > > Stephen, the change to MNG-5805 test as part of MNG-5958 was intentional, > > since I broke binary

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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: >>

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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 > >

Re: [IT MNG-5958]: Please review integration test for MNG-5958

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

[IT MNG-5958]: Please review integration test for MNG-5958

Commit to review is here: If no one objects until Friday, 13th, 2017, I'll merge it to master. Regards, -- Christian