Re: [DISCUSS] How do we want to handle false positives in the integration tests

[Michael Osipov <mi...@apache.org>]

Am 2017-01-31 um 22:25 schrieb Stephen Connolly:
> Ok so, we'll need to knock this one out and see if there is a consensus.
>
> My position is that I only have a slight preference for A over B and I
> cannot fully articulate why.
>
> Michael, do you feel you can present a reasoned argument in favour of A and
> we'll let one of the B proponents present their case and see if either side
> is "converted" to yield a consensus.

I think there should be a sweet spot between A and B. If we notice that 
the test was false positive/negative previously, we have to make it fail 
to show that this released version of Maven is broken, tell Invoker to 
expect a fail for this version range. Copy/clone this IT, modify for a 
new version of Maven expecting to work properly.

This will give you a full documentation of the previous case and the new 
(correct one). We must openly communicate that we have made a mistake to 
justify a (breaking) change in Maven for good.

Michael

> On Tue 31 Jan 2017 at 20:29, Michael Osipov <mi...@apache.org> wrote:
>
>> Am 2017-01-31 um 20:23 schrieb Stephen Connolly:
>>> Looking like a consensus on B.
>>
>> I am actually in favor of A. How do you want to assure with B that the
>> will be properly handled for current master as you fixed the test for
>> released versions?
>>
>> Michael
>>
>>> On Tue 31 Jan 2017 at 12:51, Anders Hammar <an...@hammar.net> wrote:
>>>
>>>> I favor B.
>>>>
>>>> /Anders
>>>>
>>>> On Tue, Jan 31, 2017 at 12:42 PM, Stephen Connolly <
>>>> stephen.alan.connolly@gmail.com> wrote:
>>>>
>>>>> We have kind of established a consensus on how to handle the case where
>>>> we
>>>>> want to change the specification of how Maven works going forward.
>>>>> Specifically, if we decide that the old behaviour of Maven is no longer
>>>>> going to be the new behaviour of Maven our procedure in the integration
>>>>> tests is as follows:
>>>>>
>>>>> 1. Mark the existing tests that are affected as range limited where the
>>>>> upper bound is the below the version of Maven that the change in
>>>> behaviour
>>>>> will land in
>>>>> 2. Create tests of the new behaviour (probably copied from the original
>>>>> tests but with the assertions modified and using a range limited where
>>>> the
>>>>> lower bound is the version of Maven that the change in behaviour will
>>>> land
>>>>> in.
>>>>>
>>>>> An example of such a change is
>>>>> https://github.com/apache/maven-integration-testing/commit/
>>>>> c4365abe20b58b2cbc174de812e43c7741dc10e1
>>>>>
>>>>> We now have a more complex case to try and decide how to handle, the
>>>>> current attempt to resolve is this diff:
>>>>> https://github.com/apache/maven-integration-testing/
>>>>> compare/master...MNG-2199
>>>>>
>>>>> However I am somewhat uncomfortable with how that proposed fix to the
>>>>> integration tests works.
>>>>>
>>>>> So firstly, Christian has identified that the original tests added were
>>>> not
>>>>> correctly detecting the failure.
>>>>>
>>>>> We have a situation therefore where the integration tests have been
>>>> giving
>>>>> false positive results against Maven 3.2.2+
>>>>>
>>>>> Therefore, my view is that we should *fix the broken tests* because a
>>>> false
>>>>> positive or a false negative is a bug in the tests.
>>>>>
>>>>> This would mean that the tests would no longer pass when run against
>>>>> 3.2.2-3.3.9, instead they would report the bugs in those versions that
>> we
>>>>> shipped due to the bugs in the integration tests.
>>>>>
>>>>> If we had a need to release - say security fixes - for those lines, we
>>>>> would then have to do one of:
>>>>> * ACK the continued failing tests;
>>>>> * Run with the integration tests forked from the point in time where
>> the
>>>>> previous release on the line was cut; OR
>>>>> * Back-port the fixes to those lines
>>>>>
>>>>> (assuming we are supporting those lines for security fixes)
>>>>>
>>>>> I am fine with any of those three options as those are known issues
>> that
>>>> we
>>>>> should really have JIRAs for and be documenting in the release notes,
>> and
>>>>> any of those three options would be forcing us to acknowledge the bugs.
>>>>>
>>>>> An alternative is to say "those bugs were part of the specification of
>>>>> Maven and we have changed the specification of Maven again" which is
>> the
>>>>> approach that the current MNG-2199 branch takes.
>>>>>
>>>>> I am not happy with that approach as it is an implicit approval of that
>>>>> type of usage for the broken versions of Maven. Users could
>> legitimately
>>>>> start filing feature requests to "restore" the previous behaviour
>> because
>>>>> "it was part of the specification"... fine we can probably bat those
>>>>> requests away, but is it helping us with code archeology?
>>>>>
>>>>> So, what do we want to do with the case of a test being identified as
>>>>> having either a false positive or a false negative against an already
>>>>> released version of Maven?
>>>>>
>>>>> A. Fix the test and then the test will fail against already released
>>>>> versions of Maven
>>>>> B. Fix the test, but exclude the broken versions of Maven from the
>> range
>>>>> with a comment explaining why
>>>>> C. Clone the test, leaving the broken test for the old versions of
>> Maven
>>>>> and the new test for new versions of Maven
>>>>> D. Something else
>>>>>
>>>>> I personally favour A or B (with a slight leaning towards A) and I
>> really
>>>>> do not like C for the case of the false-positive / false-negative tests
>>>>>
>>>>> If an obvious consensus does not emerge I may have to call a vote, you
>>>> have
>>>>> been warned!
>>>>>
>>>>> -Stephen
>>>>>
>>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>> --
> Sent from my phone
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: [DISCUSS] How do we want to handle false positives in the integration tests

[Michael Osipov <mi...@apache.org>]

Am 2017-02-01 um 20:05 schrieb Stephen Connolly:
> I would rather modify verifier to allow recording versions we expect to
> fail vs versions we expect to pass.
>
> I'll see if I can code it up. That would at least give us the verification
> of the failure on the broken and the verification of the fix without the
> duplication and consequent risk that users might interpret the two tests as
> a spec change

This sounds like a very good approach, but requires code change.
I would even pick up Christian's idea of tagging 
maven-integration-testing with the version of Maven (e.g., maven-3.5.0) 
to freeze tests for comparison.

> On Wed 1 Feb 2017 at 17:52, Michael Osipov <mi...@apache.org> wrote:
>
>> Am 2017-01-31 um 22:25 schrieb Stephen Connolly:
>>> Ok so, we'll need to knock this one out and see if there is a consensus.
>>>
>>> My position is that I only have a slight preference for A over B and I
>>> cannot fully articulate why.
>>>
>>> Michael, do you feel you can present a reasoned argument in favour of A
>> and
>>> we'll let one of the B proponents present their case and see if either
>> side
>>> is "converted" to yield a consensus.
>>
>> I think there should be a sweet spot between A and B. If we notice that
>> the test was false positive/negative previously, we have to make it fail
>> to show that this released version of Maven is broken, tell Invoker to
>> expect a fail for this version range. Copy/clone this IT, modify for a
>> new version of Maven expecting to work properly.
>>
>> This will give you a full documentation of the previous case and the new
>> (correct one). We must openly communicate that we have made a mistake to
>> justify a (breaking) change in Maven for good.
>>
>> Michael
>>
>>> On Tue 31 Jan 2017 at 20:29, Michael Osipov <mi...@apache.org> wrote:
>>>
>>>> Am 2017-01-31 um 20:23 schrieb Stephen Connolly:
>>>>> Looking like a consensus on B.
>>>>
>>>> I am actually in favor of A. How do you want to assure with B that the
>>>> will be properly handled for current master as you fixed the test for
>>>> released versions?
>>>>
>>>> Michael
>>>>
>>>>> On Tue 31 Jan 2017 at 12:51, Anders Hammar <an...@hammar.net> wrote:
>>>>>
>>>>>> I favor B.
>>>>>>
>>>>>> /Anders
>>>>>>
>>>>>> On Tue, Jan 31, 2017 at 12:42 PM, Stephen Connolly <
>>>>>> stephen.alan.connolly@gmail.com> wrote:
>>>>>>
>>>>>>> We have kind of established a consensus on how to handle the case
>> where
>>>>>> we
>>>>>>> want to change the specification of how Maven works going forward.
>>>>>>> Specifically, if we decide that the old behaviour of Maven is no
>> longer
>>>>>>> going to be the new behaviour of Maven our procedure in the
>> integration
>>>>>>> tests is as follows:
>>>>>>>
>>>>>>> 1. Mark the existing tests that are affected as range limited where
>> the
>>>>>>> upper bound is the below the version of Maven that the change in
>>>>>> behaviour
>>>>>>> will land in
>>>>>>> 2. Create tests of the new behaviour (probably copied from the
>> original
>>>>>>> tests but with the assertions modified and using a range limited
>> where
>>>>>> the
>>>>>>> lower bound is the version of Maven that the change in behaviour will
>>>>>> land
>>>>>>> in.
>>>>>>>
>>>>>>> An example of such a change is
>>>>>>> https://github.com/apache/maven-integration-testing/commit/
>>>>>>> c4365abe20b58b2cbc174de812e43c7741dc10e1
>>>>>>>
>>>>>>> We now have a more complex case to try and decide how to handle, the
>>>>>>> current attempt to resolve is this diff:
>>>>>>> https://github.com/apache/maven-integration-testing/
>>>>>>> compare/master...MNG-2199
>>>>>>>
>>>>>>> However I am somewhat uncomfortable with how that proposed fix to the
>>>>>>> integration tests works.
>>>>>>>
>>>>>>> So firstly, Christian has identified that the original tests added
>> were
>>>>>> not
>>>>>>> correctly detecting the failure.
>>>>>>>
>>>>>>> We have a situation therefore where the integration tests have been
>>>>>> giving
>>>>>>> false positive results against Maven 3.2.2+
>>>>>>>
>>>>>>> Therefore, my view is that we should *fix the broken tests* because a
>>>>>> false
>>>>>>> positive or a false negative is a bug in the tests.
>>>>>>>
>>>>>>> This would mean that the tests would no longer pass when run against
>>>>>>> 3.2.2-3.3.9, instead they would report the bugs in those versions
>> that
>>>> we
>>>>>>> shipped due to the bugs in the integration tests.
>>>>>>>
>>>>>>> If we had a need to release - say security fixes - for those lines,
>> we
>>>>>>> would then have to do one of:
>>>>>>> * ACK the continued failing tests;
>>>>>>> * Run with the integration tests forked from the point in time where
>>>> the
>>>>>>> previous release on the line was cut; OR
>>>>>>> * Back-port the fixes to those lines
>>>>>>>
>>>>>>> (assuming we are supporting those lines for security fixes)
>>>>>>>
>>>>>>> I am fine with any of those three options as those are known issues
>>>> that
>>>>>> we
>>>>>>> should really have JIRAs for and be documenting in the release notes,
>>>> and
>>>>>>> any of those three options would be forcing us to acknowledge the
>> bugs.
>>>>>>>
>>>>>>> An alternative is to say "those bugs were part of the specification
>> of
>>>>>>> Maven and we have changed the specification of Maven again" which is
>>>> the
>>>>>>> approach that the current MNG-2199 branch takes.
>>>>>>>
>>>>>>> I am not happy with that approach as it is an implicit approval of
>> that
>>>>>>> type of usage for the broken versions of Maven. Users could
>>>> legitimately
>>>>>>> start filing feature requests to "restore" the previous behaviour
>>>> because
>>>>>>> "it was part of the specification"... fine we can probably bat those
>>>>>>> requests away, but is it helping us with code archeology?
>>>>>>>
>>>>>>> So, what do we want to do with the case of a test being identified as
>>>>>>> having either a false positive or a false negative against an already
>>>>>>> released version of Maven?
>>>>>>>
>>>>>>> A. Fix the test and then the test will fail against already released
>>>>>>> versions of Maven
>>>>>>> B. Fix the test, but exclude the broken versions of Maven from the
>>>> range
>>>>>>> with a comment explaining why
>>>>>>> C. Clone the test, leaving the broken test for the old versions of
>>>> Maven
>>>>>>> and the new test for new versions of Maven
>>>>>>> D. Something else
>>>>>>>
>>>>>>> I personally favour A or B (with a slight leaning towards A) and I
>>>> really
>>>>>>> do not like C for the case of the false-positive / false-negative
>> tests
>>>>>>>
>>>>>>> If an obvious consensus does not emerge I may have to call a vote,
>> you
>>>>>> have
>>>>>>> been warned!
>>>>>>>
>>>>>>> -Stephen
>>>>>>>
>>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>> --
>>> Sent from my phone
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>> --
> Sent from my phone
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: [DISCUSS] How do we want to handle false positives in the integration tests

[Stephen Connolly <st...@gmail.com>]

https://git-wip-us.apache.org/repos/asf?p=maven-integration-testing.git;a=commit;h=92a11a96

(not using annotations because our integration tests are JUnit 3.8.x style)

On 1 February 2017 at 20:03, Christian Schulte <cs...@schulte.it> wrote:

> Am 02/01/17 um 20:05 schrieb Stephen Connolly:
> > I would rather modify verifier to allow recording versions we expect to
> > fail vs versions we expect to pass.
>
> +1
>
> On method level instead of class level. Maybe by employing annotations
> one can add to methods like @SupportedBy.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: [DISCUSS] How do we want to handle false positives in the integration tests

[Christian Schulte <cs...@schulte.it>]

Am 02/01/17 um 20:05 schrieb Stephen Connolly:
> I would rather modify verifier to allow recording versions we expect to
> fail vs versions we expect to pass.

+1

On method level instead of class level. Maybe by employing annotations
one can add to methods like @SupportedBy.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: [DISCUSS] How do we want to handle false positives in the integration tests

[Stephen Connolly <st...@gmail.com>]

I would rather modify verifier to allow recording versions we expect to
fail vs versions we expect to pass.

I'll see if I can code it up. That would at least give us the verification
of the failure on the broken and the verification of the fix without the
duplication and consequent risk that users might interpret the two tests as
a spec change

On Wed 1 Feb 2017 at 17:52, Michael Osipov <mi...@apache.org> wrote:

> Am 2017-01-31 um 22:25 schrieb Stephen Connolly:
> > Ok so, we'll need to knock this one out and see if there is a consensus.
> >
> > My position is that I only have a slight preference for A over B and I
> > cannot fully articulate why.
> >
> > Michael, do you feel you can present a reasoned argument in favour of A
> and
> > we'll let one of the B proponents present their case and see if either
> side
> > is "converted" to yield a consensus.
>
> I think there should be a sweet spot between A and B. If we notice that
> the test was false positive/negative previously, we have to make it fail
> to show that this released version of Maven is broken, tell Invoker to
> expect a fail for this version range. Copy/clone this IT, modify for a
> new version of Maven expecting to work properly.
>
> This will give you a full documentation of the previous case and the new
> (correct one). We must openly communicate that we have made a mistake to
> justify a (breaking) change in Maven for good.
>
> Michael
>
> > On Tue 31 Jan 2017 at 20:29, Michael Osipov <mi...@apache.org> wrote:
> >
> >> Am 2017-01-31 um 20:23 schrieb Stephen Connolly:
> >>> Looking like a consensus on B.
> >>
> >> I am actually in favor of A. How do you want to assure with B that the
> >> will be properly handled for current master as you fixed the test for
> >> released versions?
> >>
> >> Michael
> >>
> >>> On Tue 31 Jan 2017 at 12:51, Anders Hammar <an...@hammar.net> wrote:
> >>>
> >>>> I favor B.
> >>>>
> >>>> /Anders
> >>>>
> >>>> On Tue, Jan 31, 2017 at 12:42 PM, Stephen Connolly <
> >>>> stephen.alan.connolly@gmail.com> wrote:
> >>>>
> >>>>> We have kind of established a consensus on how to handle the case
> where
> >>>> we
> >>>>> want to change the specification of how Maven works going forward.
> >>>>> Specifically, if we decide that the old behaviour of Maven is no
> longer
> >>>>> going to be the new behaviour of Maven our procedure in the
> integration
> >>>>> tests is as follows:
> >>>>>
> >>>>> 1. Mark the existing tests that are affected as range limited where
> the
> >>>>> upper bound is the below the version of Maven that the change in
> >>>> behaviour
> >>>>> will land in
> >>>>> 2. Create tests of the new behaviour (probably copied from the
> original
> >>>>> tests but with the assertions modified and using a range limited
> where
> >>>> the
> >>>>> lower bound is the version of Maven that the change in behaviour will
> >>>> land
> >>>>> in.
> >>>>>
> >>>>> An example of such a change is
> >>>>> https://github.com/apache/maven-integration-testing/commit/
> >>>>> c4365abe20b58b2cbc174de812e43c7741dc10e1
> >>>>>
> >>>>> We now have a more complex case to try and decide how to handle, the
> >>>>> current attempt to resolve is this diff:
> >>>>> https://github.com/apache/maven-integration-testing/
> >>>>> compare/master...MNG-2199
> >>>>>
> >>>>> However I am somewhat uncomfortable with how that proposed fix to the
> >>>>> integration tests works.
> >>>>>
> >>>>> So firstly, Christian has identified that the original tests added
> were
> >>>> not
> >>>>> correctly detecting the failure.
> >>>>>
> >>>>> We have a situation therefore where the integration tests have been
> >>>> giving
> >>>>> false positive results against Maven 3.2.2+
> >>>>>
> >>>>> Therefore, my view is that we should *fix the broken tests* because a
> >>>> false
> >>>>> positive or a false negative is a bug in the tests.
> >>>>>
> >>>>> This would mean that the tests would no longer pass when run against
> >>>>> 3.2.2-3.3.9, instead they would report the bugs in those versions
> that
> >> we
> >>>>> shipped due to the bugs in the integration tests.
> >>>>>
> >>>>> If we had a need to release - say security fixes - for those lines,
> we
> >>>>> would then have to do one of:
> >>>>> * ACK the continued failing tests;
> >>>>> * Run with the integration tests forked from the point in time where
> >> the
> >>>>> previous release on the line was cut; OR
> >>>>> * Back-port the fixes to those lines
> >>>>>
> >>>>> (assuming we are supporting those lines for security fixes)
> >>>>>
> >>>>> I am fine with any of those three options as those are known issues
> >> that
> >>>> we
> >>>>> should really have JIRAs for and be documenting in the release notes,
> >> and
> >>>>> any of those three options would be forcing us to acknowledge the
> bugs.
> >>>>>
> >>>>> An alternative is to say "those bugs were part of the specification
> of
> >>>>> Maven and we have changed the specification of Maven again" which is
> >> the
> >>>>> approach that the current MNG-2199 branch takes.
> >>>>>
> >>>>> I am not happy with that approach as it is an implicit approval of
> that
> >>>>> type of usage for the broken versions of Maven. Users could
> >> legitimately
> >>>>> start filing feature requests to "restore" the previous behaviour
> >> because
> >>>>> "it was part of the specification"... fine we can probably bat those
> >>>>> requests away, but is it helping us with code archeology?
> >>>>>
> >>>>> So, what do we want to do with the case of a test being identified as
> >>>>> having either a false positive or a false negative against an already
> >>>>> released version of Maven?
> >>>>>
> >>>>> A. Fix the test and then the test will fail against already released
> >>>>> versions of Maven
> >>>>> B. Fix the test, but exclude the broken versions of Maven from the
> >> range
> >>>>> with a comment explaining why
> >>>>> C. Clone the test, leaving the broken test for the old versions of
> >> Maven
> >>>>> and the new test for new versions of Maven
> >>>>> D. Something else
> >>>>>
> >>>>> I personally favour A or B (with a slight leaning towards A) and I
> >> really
> >>>>> do not like C for the case of the false-positive / false-negative
> tests
> >>>>>
> >>>>> If an obvious consensus does not emerge I may have to call a vote,
> you
> >>>> have
> >>>>> been warned!
> >>>>>
> >>>>> -Stephen
> >>>>>
> >>>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> >>
> >> --
> > Sent from my phone
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
> --
Sent from my phone