From 59d4e1ab5272079150e92efba098445ff4f8aba9 Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Wed, 19 Jul 2023 13:41:14 +0100 Subject: [PATCH] fix(testresult): add skipped message to not (#189) Currently skipped tests with messages return ``, but all the other examples of JUnit I can find return ``. The inclusion of both a `` and `` confuses parsers, making them think that the test failed when it was actually skipped. Skipping tests was added in https://github.com/bazel-contrib/rules_jvm/pull/37. --- .../contrib_rules_jvm/junit5/TestResult.java | 53 ++++++++++++------- .../junit5/JunitOutputXmlTest.java | 46 ++++++++++++++-- 2 files changed, 74 insertions(+), 25 deletions(-) diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/TestResult.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/TestResult.java index 5a7cce92..c8060c40 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/TestResult.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/TestResult.java @@ -28,7 +28,9 @@ public TestResult(TestPlan testPlan, TestIdentifier id, boolean isDynamic) { public boolean isError() { TestExecutionResult result = getResult(); - if (result == null || result.getStatus() == TestExecutionResult.Status.SUCCESSFUL) { + if (result == null + || result.getStatus() == TestExecutionResult.Status.SUCCESSFUL + || isSkipped()) { return false; } @@ -37,7 +39,9 @@ public boolean isError() { public boolean isFailure() { TestExecutionResult result = getResult(); - if (result == null || result.getStatus() == TestExecutionResult.Status.SUCCESSFUL) { + if (result == null + || result.getStatus() == TestExecutionResult.Status.SUCCESSFUL + || isSkipped()) { return false; } @@ -81,28 +85,12 @@ public void toXml(XMLStreamWriter xml) { if (isDisabled() || isSkipped()) { xml.writeStartElement("skipped"); + writeThrowableMessage(xml); xml.writeEndElement(); } if (isFailure() || isError()) { - Throwable throwable = getResult().getThrowable().orElse(null); - xml.writeStartElement(isFailure() ? "failure" : "error"); - if (throwable == null) { - // Stub out the values - xml.writeAttribute("message", "unknown cause"); - xml.writeAttribute("type", RuntimeException.class.getName()); - xml.writeEndElement(); - return; - } - - xml.writeAttribute( - "message", escapeIllegalCharacters(String.valueOf(throwable.getMessage()))); - xml.writeAttribute("type", throwable.getClass().getName()); - - StringWriter stringWriter = new StringWriter(); - throwable.printStackTrace(new PrintWriter(stringWriter)); - - xml.writeCData(escapeIllegalCharacters(stringWriter.toString())); + writeThrowableMessage(xml); xml.writeEndElement(); } @@ -124,6 +112,31 @@ public void toXml(XMLStreamWriter xml) { }); } + private void writeThrowableMessage(XMLStreamWriter xml) { + write( + () -> { + Throwable throwable = null; + if (getResult() != null) { + throwable = getResult().getThrowable().orElse(null); + } + if (throwable == null) { + // Stub out the values + xml.writeAttribute("message", "unknown cause"); + xml.writeAttribute("type", RuntimeException.class.getName()); + return; + } + + xml.writeAttribute( + "message", escapeIllegalCharacters(String.valueOf(throwable.getMessage()))); + xml.writeAttribute("type", throwable.getClass().getName()); + + StringWriter stringWriter = new StringWriter(); + throwable.printStackTrace(new PrintWriter(stringWriter)); + + xml.writeCData(escapeIllegalCharacters(stringWriter.toString())); + }); + } + @Override protected Optional get(TestIdentifier id) { return getTestId().equals(id) ? Optional.of(this) : Optional.empty(); diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/JunitOutputXmlTest.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/JunitOutputXmlTest.java index 0142a762..ad99c128 100644 --- a/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/JunitOutputXmlTest.java +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/JunitOutputXmlTest.java @@ -84,6 +84,42 @@ public void skippedTestsAreNotDisabled() { result.markFinished(testExecutionResult); assertFalse(result.isDisabled()); + assertTrue(result.isSkipped()); + } + + @Test + public void skippedTestsAreNotFailures() { + TestPlan testPlan = Mockito.mock(TestPlan.class); + + TestResult result = new TestResult(testPlan, identifier, false); + TestExecutionResult testExecutionResult = + TestExecutionResult.aborted(new TestAbortedException("skipping is fun")); + result.markFinished(testExecutionResult); + + assertTrue(result.isSkipped()); + assertFalse(result.isFailure()); + assertFalse(result.isError()); + assertFalse(result.isDisabled()); + } + + @Test + public void skippedTestsContainMessages() { + var test = new TestResult(Mockito.mock(TestPlan.class), identifier, false); + test.markFinished(TestExecutionResult.aborted(new TestAbortedException("skipping is fun"))); + + var root = generateTestXml(test).getDocumentElement(); + assertNotNull(root); + assertEquals("testcase", root.getTagName()); + + var skipped = root.getElementsByTagName("skipped"); + assertEquals(1, skipped.getLength()); + + var failures = root.getElementsByTagName("failure"); + assertEquals(0, failures.getLength()); + + var message = skipped.item(0).getAttributes().getNamedItem("message"); + assertNotNull(message); + assertEquals("skipping is fun", message.getTextContent()); } @Test @@ -113,9 +149,9 @@ public void disabledTestsAreMarkedAsSkipped() { assertEquals(1, allTestCases.getLength()); Node testCase = allTestCases.item(0); - boolean skippedSeen = containsChild("skipped", testCase); + Node skipped = getChild("skipped", testCase); - assertTrue(skippedSeen); + assertNotNull(skipped); } @Test @@ -212,7 +248,7 @@ private Document generateTestXml(BaseResult result) { } } - private boolean containsChild(String childTagName, Node withinNode) { + private Node getChild(String childTagName, Node withinNode) { NodeList childNodes = withinNode.getChildNodes(); for (int i = 0; i < childNodes.getLength(); i++) { Node node = childNodes.item(i); @@ -220,10 +256,10 @@ private boolean containsChild(String childTagName, Node withinNode) { continue; } if (childTagName.equals(node.getNodeName())) { - return true; + return node; } } - return false; + return null; } private UniqueId createId(String testName) {