Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JUnit assumption violation is not reflected on Bazel UI #3476

Closed
davido opened this issue Jul 30, 2017 · 7 comments
Closed

JUnit assumption violation is not reflected on Bazel UI #3476

davido opened this issue Jul 30, 2017 · 7 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: feature request

Comments

@davido
Copy link
Contributor

davido commented Jul 30, 2017

Consider this JUnit test:

package org.gerritcon.mv2016;

import static org.junit.Assert.assertEquals;
import static org.junit.Assume.assumeTrue;

import org.junit.Before;
import org.junit.Test;

public class PrintyTest {
  @Before
  public void setUp() {
    assumeTrue(false);
  }

  @Test
  public void test() {
    assertEquals("foo", "bar");
  }
}

Running bazel test produces:

$ bazel test :printy_tests
INFO: Found 1 test target...
Target //:printy_tests up-to-date:
  bazel-bin/printy_tests.jar
  bazel-bin/printy_tests
INFO: Elapsed time: 0.764s, Critical Path: 0.59s
//:printy_tests                                                          PASSED in 0.3s

There was no mention of assumption violation(s) and the fact, that some tests were actually skipped.

For the record, here is the BUILD file:

java_test(
    name = "printy_tests",
    srcs = ["src/test/java/org/gerritcon/mv2016/PrintyTest.java"],
    test_class = "org.gerritcon.mv2016.PrintyTest",
)

Check how the assumption violation and the fact that the test was actually skipped is reported in Buck:

$ buck test :printy_tests
[-] PROCESSING BUCK FILES...FINISHED 0.0s [100%]
[-] DOWNLOADING... (0.00 B/S AVG, TOTAL: 0.00 B, 0 Artifacts)
[-] BUILDING...FINISHED 0.3s [100%] (6/6 JOBS, 4 UPDATED, 66.7% CACHE MISS)
[-] TESTING...FINISHED 0.3s (0 PASS/1 SKIP/0 FAIL)
RESULTS FOR //:printy_tests
ASSUME  <100ms  0 Passed   1 Skipped   0 Failed   org.gerritcon.mv2016.PrintyTest
TESTS PASSED (with some assumption violations)

For the record, here is the .buckconfig file:

[download]
  maven_repo = http://repo1.maven.org/maven2
  in_build = true

and here is the BUCK file:

remote_file(
    name = "junit-file",
    out = "junit.jar",
    sha1 = "4e031bb61df09069aeb2bffb4019e7a5034a4ee0",
    url = "mvn:junit:junit:jar:4.11",
)

remote_file(
    name = "hamcrest-core-file",
    sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0",
    url = "mvn:org.hamcrest:hamcrest-core:jar:1.3",
)

prebuilt_jar(
    name = "junit",
    binary_jar = ":junit-file",
)

prebuilt_jar(
    name = "hamcrest-core",
    binary_jar = ":hamcrest-core-file",
)

java_test(
    name = "printy_tests",
    srcs = ["src/test/java/org/gerritcon/mv2016/PrintyTest.java"],
    deps = [
        ":hamcrest-core",
        ":junit",
    ],
)
@dslomov
Copy link
Contributor

dslomov commented Aug 2, 2017

@iirina can you take a look? seems like a useful feature, but need priority

@iirina iirina added the P3 We're not considering working on this, but happy to review a PR. (No assignee) label Aug 2, 2017
@iirina
Copy link
Contributor

iirina commented Aug 2, 2017

I will take a look this or next week and see if we can get away with a quick solution. If not, this will have to wait a bit more, as other things have higher priority right now.

@davido
Copy link
Contributor Author

davido commented May 22, 2018

Any updates on this?

Interestingly, I see that assumption violation is tested in Bazel test, both on suite and test level:

@Test
public void assumptionViolationsAreReportedAsSkippedTests() throws Exception {
TestSuiteModelSupplier mockModelSupplier = mock(TestSuiteModelSupplier.class);
TestSuiteModel mockModel = mock(TestSuiteModel.class);
CancellableRequestFactory mockRequestFactory = mock(CancellableRequestFactory.class);
OutputStream mockXmlStream = mock(OutputStream.class);
JUnit4TestXmlListener listener = new JUnit4TestXmlListener(
mockModelSupplier, mockRequestFactory, fakeSignalHandlers, mockXmlStream, errPrintStream);
Request request = Request.classWithoutSuiteMethod(TestWithAssumptionViolation.class);
Description suiteDescription = request.getRunner().getDescription();
Description testDescription = suiteDescription.getChildren().get(0);
when(mockModelSupplier.get()).thenReturn(mockModel);
JUnitCore core = new JUnitCore();
core.addListener(listener);
core.run(request);
assertWithMessage("no output to stderr expected").that(errStream.size()).isEqualTo(0);
InOrder inOrder = inOrder(mockModel);
inOrder.verify(mockModel).testSkipped(testDescription);
inOrder.verify(mockModel).writeAsXml(mockXmlStream);
verify(mockRequestFactory, never()).cancelRun();
}
@Test
public void assumptionViolationsAtSuiteLevelAreReportedAsSkippedSuite() throws Exception {
TestSuiteModelSupplier mockModelSupplier = mock(TestSuiteModelSupplier.class);
TestSuiteModel mockModel = mock(TestSuiteModel.class);
CancellableRequestFactory mockRequestFactory = mock(CancellableRequestFactory.class);
OutputStream mockXmlStream = mock(OutputStream.class);
JUnit4TestXmlListener listener = new JUnit4TestXmlListener(
mockModelSupplier, mockRequestFactory, fakeSignalHandlers, mockXmlStream, errPrintStream);
Request request = Request.classWithoutSuiteMethod(
TestWithAssumptionViolationOnTheSuiteLevel.class);
Description suiteDescription = request.getRunner().getDescription();
when(mockModelSupplier.get()).thenReturn(mockModel);
JUnitCore core = new JUnitCore();
core.addListener(listener);
core.run(request);
assertWithMessage("no output to stderr expected").that(errStream.size()).isEqualTo(0);
InOrder inOrder = inOrder(mockModel);
inOrder.verify(mockModel).testSkipped(suiteDescription);
inOrder.verify(mockModel).writeAsXml(mockXmlStream);
verify(mockRequestFactory, never()).cancelRun();
}

@davido
Copy link
Contributor Author

davido commented May 27, 2018

OK, I see now what happens. The JUnit4TestXmlListener from my previous comment is irrelevant for Bazel's UI test outcome reporting.

A Birds Eye View of Bazel's UI rendering <-> JUnit Runner integration:

  1. Bazel test command is called and spawns testrunner process
  2. Test runner command invokes JUnit core. JUnit's core standard text UI creates the test.log file and exits
  3. Bazel test command parses test.log file and renders the Bazel UI test report

Given that standard JUnit text UI doesn't currently reflect assumption violations, this information is lost.

How it works in Buck? From the very first commit, back in 2013, Buck's java_test rule uses custom XML format for communication between JUnit testrunner process and Buck daemon, instead of relying on the (limited) JUnit text UI. This is the commit that introduced assumption violation in Buck UI. As it can be seen, XML status attribute is uniquely identifying the assumption violation outcome: ASSUMPTION_VIOLATION:

<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<testcase name="org.gerritcon.mv2016.PrintyTest" runner_capabilities="simple_test_selector">
    <test message="got: &lt;false&gt;, expected: is &lt;true&gt;" name="test_assumption_violation" stacktrace="org.junit.AssumptionViolatedException: got: &lt;false&gt;, expected: is &lt;true&gt;&#10;&#9;at org.junit.Assume.assumeThat(Assume.java:95)&#10;&#9;at org.junit.Assume.assumeTrue(Assume.java:41)&#10;&#9;at org.gerritcon.mv2016.PrintyTest.test_assumption_violation(PrintyTest.java:26)&#10;&#9;at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)&#10;&#9;at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)&#10;&#9;at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)&#10;&#9;at java.lang.reflect.Method.invoke(Method.java:498)&#10;&#9;at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)&#10;&#9;at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)&#10;&#9;at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)&#10;&#9;at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)&#10;&#9;at com.facebook.buck.testrunner.SameThreadFailOnTimeout.lambda$new$0(SameThreadFailOnTimeout.java:44)&#10;&#9;at java.util.concurrent.FutureTask.run(FutureTask.java:266)&#10;&#9;at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)&#10;&#9;at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)&#10;&#9;at java.lang.Thread.run(Thread.java:748)&#10;" success="false" suite="org.gerritcon.mv2016.PrintyTest" time="5" type="ASSUMPTION_VIOLATION"/>
    <test message="expected:&lt;[foo]&gt; but was:&lt;[bar]&gt;" name="test_failed" stacktrace="org.junit.ComparisonFailure: expected:&lt;[foo]&gt; but was:&lt;[bar]&gt;&#10;&#9;at org.junit.Assert.assertEquals(Assert.java:115)&#10;&#9;at org.junit.Assert.assertEquals(Assert.java:144)&#10;&#9;at org.gerritcon.mv2016.PrintyTest.test_failed(PrintyTest.java:21)&#10;&#9;at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)&#10;&#9;at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)&#10;&#9;at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)&#10;&#9;at java.lang.reflect.Method.invoke(Method.java:498)&#10;&#9;at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)&#10;&#9;at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)&#10;&#9;at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)&#10;&#9;at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)&#10;&#9;at com.facebook.buck.testrunner.SameThreadFailOnTimeout.lambda$new$0(SameThreadFailOnTimeout.java:44)&#10;&#9;at java.util.concurrent.FutureTask.run(FutureTask.java:266)&#10;&#9;at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)&#10;&#9;at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)&#10;&#9;at java.lang.Thread.run(Thread.java:748)&#10;" success="false" suite="org.gerritcon.mv2016.PrintyTest" time="1" type="FAILURE"/>
    <test name="test_passed" success="true" suite="org.gerritcon.mv2016.PrintyTest" time="0" type="SUCCESS"/>
</testcase>

FTR: Here is a working Buck's JUnit test example with assumption violations:

$ buck test :printy_tests
FAILURE org.gerritcon.mv2016.PrintyTest test_failed: expected:<[foo]> but was:<[bar]>
[-] PROCESSING BUCK FILES...FINISHED 0.0s 🏖  (Watchman reported no changes)
[-] DOWNLOADING... (0.00 B/S AVG, TOTAL: 0.00 B, 0 Artifacts)
[-] BUILDING...FINISHED 0.0s [100%] (6/6 JOBS, 0 UPDATED, 0 [0.0%] CACHE MISS)
[-] TESTING...FINISHED 0.0s (1 PASS/1 SKIP/1 FAIL)
RESULTS FOR //:printy_tests
FAIL    CACHED  1 Passed   1 Skipped   1 Failed   org.gerritcon.mv2016.PrintyTest

lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue May 28, 2018
Testcontainers is a Java 8 library that supports JUnit tests, providing
lightweight, throwaway instances of common databases, Selenium web
browsers, or anything else that can run in a Docker container.

This change replaces ES integration test that currently uses embedded
ES mode with ES docker image using Testcontainers library. All this is
done from within acceptance tests.

This change removes dependency on ES server stack for the test code. As
the consequence, this stack can be removed.

Prerequisite for this change is installed docker service on the SUT. If
docker service is not installed, assumption violation is raised so that
the tests don't fail. Unfortunately, due to this missing Bazel feature,
JUnit assumption violations are not reflected on the Bazel UI: [1] yet.

[1] bazelbuild/bazel#3476

Change-Id: Iccf44310292cc44bff9173f2a4ea757b43f77183
@kcooney
Copy link

kcooney commented Jun 1, 2018

@davido I would be surprised if Bazel is reading the output of JUnit's text UI.

My understanding is that the output from Bazel is reporting the status of test actions, not test methods. Bazel supports many kinds of test actions, and most of them don't have a concept of assumption failures. So Bazel looks at the exit value of the test process to determine the pass/fail status of the test action.

FYI: I don't work on Bazel–so I can't fix the issue–but I worked on the JUnit integration of Blaze and that code was refactored and used in Bazel.

@kcooney
Copy link

kcooney commented Jun 1, 2018

BTW you can get tell Bazel to provide more detailed ouput of the test (see https://docs.bazel.build/versions/master/user-manual.html#flag--test_summary). Note that from Bazel's perspective–and JUnit's–this test passed so it might not show details unless you requested verbose output. Try a test with one failing test and one assumption failure.

lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Jun 4, 2018
Testcontainers is a Java 8 library that supports JUnit tests, providing
lightweight, throwaway instances of common databases, Selenium web
browsers, or anything else that can run in a Docker container.

This change replaces ES integration test that currently uses embedded
ES mode with ES docker image using Testcontainers library. All this is
done from within acceptance tests.

This change removes dependency on ES server stack for the test code. As
the consequence, this stack can be removed.

Prerequisite for this change is installed docker service on the SUT. If
docker service is not installed, assumption violation is raised so that
the tests don't fail. Unfortunately, due to this missing Bazel feature,
JUnit assumption violations are not reflected on the Bazel UI: [1] yet.

[1] bazelbuild/bazel#3476

Change-Id: Iccf44310292cc44bff9173f2a4ea757b43f77183
openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jun 10, 2018
Add separate test classes to test queries with Elasticsearch version 6.

The entire test classes are basically copied from the existing ones,
with the only difference being the version passed into the config and
into the container creation. This cannot be achieved using an abstract
class due to the container being created in the @BeforeClass annotated
method which is static and cannot be overridden by a derived class.

The docker image for V6 testing requires this limit [1] to be secured.
The test container otherwise aborts upon running V6 tests, causing an
AssumptionViolatedException, which is currently ignored by Bazel ([2]).
Amend the test execution documentation accordingly.

Replace the V6 image used for testing with the oss one, which has no
X-Pack or security defaults installed; cf. [3]. Without this change,
V6 tests fail upon api usage lacking authorization.

[1] https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html
[2] bazelbuild/bazel#3476
[3] https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html

Bug: Issue 9112
Bug: Issue 9212
Change-Id: I11ced5f811cdc078d7feba187086e803326886cb
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Dec 5, 2019
JUnit assumption violation is not reflected on Bazel UI: [1]. As the
consequence, if a test is passing it's not clear, it's skipped due to
assumption violation or it is passing the actual tests. Especially for
the important Gerrit features like git wire protocol v2 we must be sure
that the integration tests got actually executed and were successful.

Given that the git wire protocol test requires very recent git client
version (2.18) add git-protocol-v2 label to the test rule and provide
instructions how to skip the test in Bazel documentation.

[1] bazelbuild/bazel#3476

Bug: Issue 12032
Change-Id: Icba99ab22fa7e99fb303eb79cf3df3b354ee3d77
@meisterT meisterT added team-Rules-Java Issues for Java rules and removed category: rules > java labels May 12, 2020
lucamilanesio pushed a commit to lucamilanesio/index-elasticsearch that referenced this issue May 13, 2022
Testcontainers is a Java 8 library that supports JUnit tests, providing
lightweight, throwaway instances of common databases, Selenium web
browsers, or anything else that can run in a Docker container.

This change replaces ES integration test that currently uses embedded
ES mode with ES docker image using Testcontainers library. All this is
done from within acceptance tests.

This change removes dependency on ES server stack for the test code. As
the consequence, this stack can be removed.

Prerequisite for this change is installed docker service on the SUT. If
docker service is not installed, assumption violation is raised so that
the tests don't fail. Unfortunately, due to this missing Bazel feature,
JUnit assumption violations are not reflected on the Bazel UI: [1] yet.

[1] bazelbuild/bazel#3476

Change-Id: Iccf44310292cc44bff9173f2a4ea757b43f77183
lucamilanesio pushed a commit to lucamilanesio/index-elasticsearch that referenced this issue May 13, 2022
Testcontainers is a Java 8 library that supports JUnit tests, providing
lightweight, throwaway instances of common databases, Selenium web
browsers, or anything else that can run in a Docker container.

This change replaces ES integration test that currently uses embedded
ES mode with ES docker image using Testcontainers library. All this is
done from within acceptance tests.

This change removes dependency on ES server stack for the test code. As
the consequence, this stack can be removed.

Prerequisite for this change is installed docker service on the SUT. If
docker service is not installed, assumption violation is raised so that
the tests don't fail. Unfortunately, due to this missing Bazel feature,
JUnit assumption violations are not reflected on the Bazel UI: [1] yet.

[1] bazelbuild/bazel#3476

Change-Id: Iccf44310292cc44bff9173f2a4ea757b43f77183
lucamilanesio pushed a commit to lucamilanesio/index-elasticsearch that referenced this issue May 13, 2022
Add separate test classes to test queries with Elasticsearch version 6.

The entire test classes are basically copied from the existing ones,
with the only difference being the version passed into the config and
into the container creation. This cannot be achieved using an abstract
class due to the container being created in the @BeforeClass annotated
method which is static and cannot be overridden by a derived class.

The docker image for V6 testing requires this limit [1] to be secured.
The test container otherwise aborts upon running V6 tests, causing an
AssumptionViolatedException, which is currently ignored by Bazel ([2]).
Amend the test execution documentation accordingly.

Replace the V6 image used for testing with the oss one, which has no
X-Pack or security defaults installed; cf. [3]. Without this change,
V6 tests fail upon api usage lacking authorization.

[1] https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html
[2] bazelbuild/bazel#3476
[3] https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html

Bug: Issue 9112
Bug: Issue 9212
Change-Id: I11ced5f811cdc078d7feba187086e803326886cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants
@davido @meisterT @kcooney @iirina @dslomov and others