Interrupted test cases cause random failures with thread reuse

[terorontti]

Whenever a test case is interrupted (Thread.interrupt()), the test runner thread seems to stay in interrupted state. This causes random failures later when the thread is reused for other test cases.

Expected behaviour: When the thread is reused, interrupted flag for the thread is cleaned before starting a new test case. This can be done by calling Thread.interrupted(), which returns the interrupted state, and clears the interrupted flag.

The following code should reproduce the problem, given that test1 is run before test2

import org.junit.Test;

public class ThreadTest {
  @Test
  public void test1() {
    Thread.currentThread().interrupt();
  }

  @Test
  public void test2() throws InterruptedException {
    Thread.sleep(1000);
  }
}

The problem I encountered is similar to the one described in https://www.thoughtwire.com/junit-interupt/

[kcooney]

Thanks for the bug report.

I wonder if JUnit should cause an otherwise passing test to fail if the interrupted bit was set during the test run. I am guessing no, because so many otherwise passing tests would start failing, and the fix would be to sprinkle interrupted() calls everywhere.

There is also the question of whether ExpectesException assertThrows() or expectThrows() should ever clear the interrupted bit. I am guessing they should.

@junit-team/junit-committers thoughts?

[ArneDeutsch]

I would support the idea to call Thread.interrupted() on each thread that is reused for more than one test case just before executing a new test case. But especially if one thread is used for more then one test class. The interrupted flag is state that leaks from one test case to others, making test cases flaky. Especially the test cases that deal with multiple threads that are notoriously hard to get right anyway.

[npathai]

@kcooney I can start working on this.

[kcooney]

@npathai fantastic!

There is still an open question of whether ExpectedException or assertThrows() should ever clear the interrupted bit. @junit-team/junit-committers thoughts?

[stefanbirkner]

IMHO we should clear the interrupted flag after each test. We should not clear it in ExpectedException and assertThrows() because it will cleared after the test.

@kcooney can you please explain why you think that ExpectedException and assertThrows() should clear the flag.

[kcooney]

@stefanbirkner the general guidance I've read is any time you catch InterruptedException and don't rethrow it you should call interrupt (see https://www.ibm.com/developerworks/library/j-jtp05236/index.html and search for "Don't swallow interrupts"). Since ExpectedException and assertThrows() "swallow" unexpected exceptions, I wanted to see what people thought we should do.

[kcooney]

FWIW, I did a quick search in GitHub for calls to interrupted() in JUnit 5 and I didn't find any calls.

[npathai]

@kcooney @stefanbirkner I was wondering how IDE's integrate with JUnit framework. If I have long running test case something like

@Test
  public void test1() throws InterruptedException {
    Thread.sleep(100000);
  }

@Test
  public void test2() throws InterruptedException {
    Thread.sleep(100000);
  }

and I stop the test case forcefully using IDE. Will the IDE interrupt the Thread running JUnit tests or something else?
I checked the behavior in IntelliJ, it seems that they run different process and interrupt the process when test cases are stopped forcefully from IDE. I see Process finished with exit code 130 (interrupted by signal 2: SIGINT) in the console.
I went through eclipse JDT source and found that they use some sort of remote connection for execution of test cases.

My doubt is in a way we are violating Thread.interrupted() convention by not honoring the interruption and swallowing it. So I thought if IDE integration actually used junitThread.interrupt() to stop further execution of JUnit tests then it will not honor that request. So in this case when test1 is executing and I click stop button from IDE, it should allow current method to execute and not run further test cases. Will this behavior get affected by resetting interrupt status flag.

I apologize if the question is dumb, I don't have experience with how IDE's integrate. It will be great if you can shed some light for me.

[kcooney]

@ npathai I think IDEs call RunNotifier.pleaseStop()

[kcooney]

So it sounds like the concerns is this: what happens if the IDE calls pleaseStop() then Thread.interrupt() on the thread running the tests?

If that thread is not in a blocking method and doesn't call a blocking method, the the test run will abort after the current test complets (due to pleaseStop()).

If that thread is in a blocking method or calls a blocking method, it will throw InterruptedException. If the test doesn't catch that exception it will fail, then the test run will abort.

If the test swallows the exception things get tricky.

[Tibor17]

We use pleaseStop() in Surefire. We rely on current behavior that the JUnit's Scheduler stops scheduling a new Runnable (test method) but the pending method goes on.

…

On Fri, Nov 9, 2018 at 5:48 PM Kevin Cooney ***@***.***> wrote: So it sounds like the concerns is this: what happens if the IDE calls pleaseStop() then Thread.interrupt() on the thread running the tests? If that thread is not in a blocking method and doesn't call a blocking method, the the test run will abort after the current test complets (due to pleaseStop() If that thread is in a blocking method or calls a blocking method, it will throw InterruptedException. If the test doesn't catch that exception it will fail, then the test run will abort. If the test swallows the exception things get tricky. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1365 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_yR9IgJF3rTXtFz9y9EIBig914twYYks5utbHrgaJpZM4KIg2k> .

-- Cheers Tibor

[joaodias14]

Shouldn't this be cherry-picked to JUnit 5? I'm having the same issue with 5.3.2.

[marcphilipp]

@joaodias14 Please upgrade to the latest version, it was fixed in 5.4.0 (see junit-team/junit5#1688).

[joaodias14]

Many thanks @marcphilipp!