ParallelRunner doesn't recognize assumptions

[christian-draeger]

hey, it seems to me that the ParallelRunner is having some issue with assumeTrue / assumeThat

i tryed something like

public class MyTest {
    @Test
    public void checkSomeThing() {
        assumeTrue(false);
        System.out.println("hello");
    }
}

if i run this test with ParallelRunner test is green and "hello" is printet to console. Without ParallelRunner test will be skipped as expected.

greetings
Chris

[grthr]

I had the exact same problem of unit tests not being skipped with AssumptionViolatedException.
It turned out to be an issue of ParallelRunner as it works correctly with the default JUnit runner.

[MichaelTamm]

Hmm, I can not reproduce the bug.

I can confirm, that the test is marked as green and not as skipped -- which is hard too fix, because that's how JUnit 4 behaves by default, see: https://github.com/junit-team/junit4/blob/master/src/main/java/org/junit/runner/Result.java#L140

Nonetheless, assumeTrue(false) will still throw an AssumptionViolatedException and therefore System.out.println("hello") is not executed.

[grthr]

Ok, that's exactly what I ment. I did not try out if "hello" is printed out in this example.

But I have to disagree with you about the default behaviour of JUnit 4. Tests throwing an AssumptionViolatedException are marked as skipped like those marked with the @Ignore annotation. I have JUnit 4.12 and tried this with Eclipse and IntelliJ IDEA.

I suppose the handling of AssumptionViolatedException is different for @Test and @Theory in JUnit.

[MichaelTamm]

Eclipse / IntelliJ IDEA improve the default behaviour of JUnit. Nonetheless, that is a feature of Eclipse / IntelliJ IDEA and the default behaviour of JUnit is to ignored any AssumptionViolatedException.

I agree, that it would be nice, if skipped tests executed using the ParallelRunner would be marked as skipped when executed in a IDE, but unfortunately I don't know how to fix that.

[grthr]

Sorry, but I cannot agree with that. In a JUnit @Test the AssumptionViolatedException is handled specially, see: https://github.com/junit-team/junit4/blob/master/src/main/java/org/junit/runners/ParentRunner.java#L328

But this code does obviously not run using the ParallelRunner, as Theories are behaving differently. In Theories, the AssumptionViolatedException leads to adding the current parameters of the Theory to the list of invalid parameters. This is due to the design of parameterized tests in JUnit but should not be generalized for JUnit as a whole.

[MichaelTamm]

"... does obviously not run ..." -- Really? Did you set a breakpoint in the runLeaf method and executed a test annotated with @RunWith(ParallelRunner.class) to make sure you are right?

I guess not, because the runLeaf method is executed!

Please do a debug session and step into the methods starting from the breakpoint in the runLeaf method to see what is really happening ...

[grthr]

Well, I don't know what you are debugging, but the AssumptionViolatedException is catched and not rethrown here: https://github.com/junit-team/junit4/blob/master/src/main/java/org/junit/experimental/theories/Theories.java#L235

Thereby line 328 of ParentRunner will never execute in a Theory. The behavior is different in a standard Test, as the AssumptionViolatedException is handled in ParentRunner only.

[MichaelTamm]

Sorry, my fault -- I can now reproduce, that line 327 in ParentRunner.java ( eachNotifier.addFailedAssumption(e);) is not executed when a @Test method throws an AssumptionViolatedException.

Nonetheless: This is the current behavior of the Theories runner, from which ParallelRunner is derived.

Therefore I think, this issue should be reported (and fixed) in the https://github.com/junit-team/junit4 project.

I don't think it is a good idea to change the handling of AssumptionViolatedExceptions in ParallelRunner, because it should be interchangeable with the Theories runner.

[grthr]

As already said, this is not a bug of the Theories runner but a feature which is described here: https://github.com/junit-team/junit4/wiki/Theories

UserTest will attempt to run filenameIncludesUsername on every compatible DataPoint defined in the class. If any of the assumptions fail, the data point is silently ignored. If all of the assumptions pass, but an assertion fails, the test fails.

I appreciate that you want to keep your runner interchangeable with the Theories runner. But your runner needs to handle more cases as it also runs for non-theory tests with the @Test annotation (which the Theories runner doesn't need to handle). If you want that runner to be compatible with @Test and @Theory, a solution could be to handleAssumptionViolatedException differently depending on the current annotation of the test.

[MichaelTamm]

That sounds like a sensible idea to me (although I still think that behavior belongs into the Theories runner, because the Theories runner can handle @Test methods).

Would you like to update your pull request accordingly? And if so, please update ParallelRunnerTest too.

[christian-draeger]

hey,
i think @efzwo's idea to separate the behaviour of @test and @theory is good. since i'm not sure how to implement it easily (otherwise i would send a pull request) it would be awesome if one of you could do that. thanks in advance and thanks for the nice project

[grthr]

I've updated my pull request accordingly.

[grthr]

@MichaelTamm Could you please create a Maven package for v2.3?

[MichaelTamm]

JUnit Toolbox 2.3 is in the central Maven repo now.