search

gradle / test-retry-gradle-plugin

Gradle plugin to retry tests that have failed to mitigate test flakiness.
Apache License 2.0
222 stars 50 forks source link

Fix issue with test event not being generated #297

Closed ghale closed 2 months ago

ghale commented 2 months ago

This is a fix for the issue raised in https://github.com/gradle/gradle/issues/29633.

Build operations for test events are handled in Gradle by registering a TestListenerInternal instance that tracks running tests and generates a build operation when a test starts/stops. We have a hierarchy of operations similar to the following:

Test task
    Test task action 
        Test run 
             Test execution (1..n)
                 Test class (1..n)
                     Test method (1..n)

When we have an issue with initializing a test during a retry, we end up in a situation where the retry test result processor thinks that another retry is possible, so it does not notify its delegate that the test run is complete. However, the test retry executer sees that there were tests that were marked for retry, but were not retried, and throws an exception. The result of this is that the "test run" build operation is never completed, causing the "dangling build operation" failure.

Reproducing the behavior in the original report is problematic as it requires the develocity plugin which applies its own version of the retry plugin. So the reproducer test only checks that the retry plugin is always calling the started/completed methods for the TestListenerInternal broadcaster for every test descriptor (including the one for the overall test run).

pshevche commented 2 months ago

Blocked until https://github.com/gradle/test-retry-gradle-plugin/pull/296 is merged. The test will likely have to go away as the sample won't result in unretried tests anymore. I am not sure how to reproduce it otherwise, though.

ghale commented 2 months ago

One thing that might work would be a JUnit Jupiter parameterized test with a @MethodSource annotation, where the method checks a static counter, or the presence of a file or something and throws an exception on the second execution. I haven't tried this, but it seems like it would cause a similar failure where the test can't be initialized.

Another option would be to turn it into a unit test and recreate the failure that way, but the behavior is so tied in to that of the executer, it might not be as effective.

I don't think it's a big deal if you get rid of the test, but it would be nice to validate that the retry processor properly interacts with its delegate to future-proof the behavior.

pshevche commented 2 months ago

One thing that might work would be a JUnit Jupiter parameterized test with a @MethodSource annotation, where the method checks a static counter, or the presence of a file or something and throws an exception on the second execution. I haven't tried this, but it seems like it would cause a similar failure where the test can't be initialized.

Another option would be to turn it into a unit test and recreate the failure that way, but the behavior is so tied in to that of the executer, it might not be as effective.

I don't think it's a big deal if you get rid of the test, but it would be nice to validate that the retry processor properly interacts with its delegate to future-proof the behavior.

This, unfortunately, did not work. We can retry failures in the param providers. I experimented a bit with dynamic tests and so on, but with no luck. I'll take another look at it after the release. We need to get it to the customer soon.