You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by seanf <gi...@git.apache.org> on 2015/12/02 04:53:28 UTC

[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

GitHub user seanf opened a pull request:

    https://github.com/apache/maven-surefire/pull/108

    [SUREFIRE-1202] Allow rerunFailingTestsCount, skipAfterFailureCount t…

    …ogether

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/seanf/maven-surefire SUREFIRE-1202

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/maven-surefire/pull/108.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #108
    
----
commit 6d689fb9fade2618a9fc3ce8cfc996e771aca35c
Author: Sean Flanigan <sf...@redhat.com>
Date:   2015-12-02T03:50:04Z

    [SUREFIRE-1202] Allow rerunFailingTestsCount, skipAfterFailureCount together

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-161337141
  
    Try to build IT with your code with more realistic counts, but the fail-fast feature would not be coherent due to concurrency problem across forks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-167269576
  
    @seanf done!
    You can check it out with 2.20-SNAPSHOT.
    I will start the Release Vote on Sunday/Monday.
    2.19.1 will be in Maven Central on Wednesday.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-161945817
  
    @seanf 
    >>Are you saying you did try to build an IT for this?
    reuseForks=false would not work in 2.19 feature.
    Initially skipAfterFailureCount was not count because it was boolean and therefore rerunFailingTestsCount did not make sense.
    Now it may have sense.
    
    >>What counts would you consider realistic?
    Maybe 5 test classes or more with Thread.sleep() 3 seconds, forkCount=2, rerunFailingTestsCount = 3 and skipAfterFailureCount=2. There should be two tests. One should work with #failures < rerunFailingTestsCount (all 5 test classes passed with flaky tests), and second test in IT where #failures > rerunFailingTestsCount (totally maybe only 3 or 4 test classes run and two failed). Each class has one method.
    The classes are ordered on alphabetical order and forked.
    That's my idea but you can improve it of course and the system properties can be used in order to force tests to fail e.g.
    ```
     @Test test() {
      if (sys-prop) Assert.fail();
    }
    ```
    Let's see how long sleep() should be in the tests..
    
    >>I'm still assuming failureCount is meant to count failed tests, not bad
    runs. You didn't answer my question about that (and the docs don't say),
    but I do think it is better to count tests, as long as it is documented.
    
    If the rerun is enabled, the flaky tests (I mean those bad tests < rerunFailingTestsCount) should not be a subject to the feature with skipAfterFailureCount. I don't remember the code in detail. We need the IT to confirm both features may work together.
    
    >>As I said, there should be no race condition if failureCount counts
    (completely) failed tests, assuming the runs for each test are serialised.
    Race condition is between forks. In extremely short test method race condition may happen and the failure count may grow over expected skipAfterFailureCount .



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-162313183
  
    @seanf I think your feature should be possible only in `surefire-junit4` provider. Not in `surefire-junit47`. Did you have time to run tests with your implementation?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by seanf <gi...@git.apache.org>.
Github user seanf commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-164965305
  
    Okay, thanks @Tibor17 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by seanf <gi...@git.apache.org>.
Github user seanf closed the pull request at:

    https://github.com/apache/maven-surefire/pull/108


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by seanf <gi...@git.apache.org>.
Github user seanf commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-161909472
  
    Hi Tibor,
    
    I don't understand. Are you saying you did try to build an IT for this?
    Could you share it, please? What counts would you consider realistic?
    
    I won't have any time to look at the code for a few days, but running a
    test more than once *in series* (not parallel) should not affect
    concurrency, because the test hasn't failed until all runs for that test
    have finished.
    
    I'm still assuming failureCount is meant to count failed tests, not bad
    runs. You didn't answer my question about that (and the docs don't say),
    but I do think it is better to count tests, as long as it is documented.
    
    As I said, there should be no race condition if failureCount counts
    (completely) failed tests, assuming the runs for each test are serialised.
    
    Sean.
    
    On 3 December 2015 at 01:38, Tibor Digana <no...@github.com> wrote:
    
    > Try to build IT with your code with more realistic counts, but the
    > fail-fast feature would not be coherent due to concurrency problem across
    > forks.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/maven-surefire/pull/108#issuecomment-161337141>
    > .
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-161285312
  
    @seanf Can you edd integration test?
    You can add new test method to an existing IT.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] maven-surefire pull request: [SUREFIRE-1202] Allow rerunFailingTes...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the pull request:

    https://github.com/apache/maven-surefire/pull/108#issuecomment-164952250
  
    @seanf Please close this PR. I will push the fix to the master. Thx for contributing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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