maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anders Hammar <and...@hammar.net>
Subject Re: [DISCUSS] How do we want to handle false positives in the integration tests
Date Tue, 31 Jan 2017 12:51:31 GMT
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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message