spock icon indicating copy to clipboard operation
spock copied to clipboard

Flaky test or race condition - `ParallelSpec#@ResourceLock with only READ allows parallel execution of data-driven features`

Open Vampire opened this issue 2 years ago • 6 comments

Describe the bug

I've seen the ParallelSpec#@ResourceLock with only READ allows parallel execution of data-driven features test failing with

    Condition not satisfied:

    atomicInteger.get() == 3
    |             |     |
    3             1     false
        at apackage.ASpec.writeA(Script_9186d4f70d72d15ebba0478d4c746046.groovy:14)

in master lately in GitHub Action run.

I tried to reproduce by adding @RepeatUntilFailure (nice addition). First try didn't reproduce in about 1000 runs. Second try reproced after 4 iterations with the same failure. Third try didn't reproduce again in about 1000 runs.

So it seems there is some race condition somewhere.

To Reproduce

Add @RepeatUntilFailure to the test and try to make it fail

Expected behavior

No assertion error

Actual behavior

Occasionally assertion error

Java version

Seen with Java 8 and Java 17. Seen with Groovy 2.5 and Groovy 3.0 variant.

Buildtool version

Gradle as configured in master

What operating system are you using

Windows

Vampire avatar Mar 13 '23 11:03 Vampire

I wonder how get() can return 1 when toString() returned 3 actually.

Vampire avatar Mar 13 '23 12:03 Vampire

I wonder how get() can return 1 when toString() returned 3 actually.

Interesting question. Maybe the value has changed in between the evaluations of both? (No, I have not thought this through yet, just guessing in "rubber duck mode".)

kriegaex avatar Mar 13 '23 12:03 kriegaex

Yeah, but it "shouldn't" be possible :tm: It is an AtomicInteger that is incremented with incrementAndGet() three times. And there is a CountDownLatch that waits for the three increments to finish. And if the CountDownLatch would time out while waiting, there should be an InterruptedException, not this behavior, shouldn't it? :-/

Damn you, multi-thread development. :-D

Vampire avatar Mar 13 '23 14:03 Vampire

I even quickly added a noExceptionThrown() now to make sure it is not only masked, but I still got the assertion error.

Vampire avatar Mar 13 '23 14:03 Vampire

Do you think that maybe in rare cases countDownLatch.await(timeout, MILLISECONDS) might time out? Maybe you want to assert on the return value and check if it is false in the problematic case.

https://github.com/spockframework/spock/blob/3d7aa638558f1c4f04e09ebbfd0d3d9b7b975fe8/spock-specs/src/test/groovy/org/spockframework/smoke/extension/ParallelSpec.groovy#L518-L524

Sorry, I did not run or even check out the code, I just had 3 minutes while waiting for a chat reply while working.

kriegaex avatar Mar 13 '23 14:03 kriegaex

Oh, you might be right, thanks. Now that I double-checked I see that await returns false if the timeout occurs. I thought it will throw an exception in that case, but that's not the case. That would be good then, as it then is a test-bug, not a code bug. :-)

Vampire avatar Mar 13 '23 14:03 Vampire