You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "dr29bart (via GitHub)" <gi...@apache.org> on 2024/01/11 16:05:52 UTC
[PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
dr29bart opened a new pull request, #716:
URL: https://github.com/apache/maven-surefire/pull/716
> A regression bug appeared in 3.0.0-M6:
>
> A testNG test class has a listener which updates results from SUCCESS to FAILURE:
``` java
@Override
public void onTestSuccess(ITestResult result) {
result.setStatus(ITestResult.FAILURE);
result.getTestContext().getPassedTests().removeResult(result);
result.getTestContext().getFailedTests().addResult(result);
}
```
> Surefire fails to process a failed test result without a throwable and reports 0 total tests.
```
ForkStarter IOException: java.util.NoSuchElementException.
org.apache.maven.plugin.surefire.booterclient.output.MultipleFailureException: java.util.NoSuchElementException
at org.apache.maven.plugin.surefire.booterclient.output.ThreadedStreamConsumer$Pumper.<init>(ThreadedStreamConsumer.java:59)
at org.apache.maven.plugin.surefire.booterclient.output.ThreadedStreamConsumer.<init>(ThreadedStreamConsumer.java:107)
at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:546)
at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:285)
at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:250)
...
Suppressed: java.util.NoSuchElementException
at java.base/java.util.StringTokenizer.nextToken(StringTokenizer.java:347)
at org.apache.maven.plugin.surefire.report.StatelessXmlReporter.getTestProblems(StatelessXmlReporter.java:454)
at org.apache.maven.plugin.surefire.report.StatelessXmlReporter.serializeTestClassWithoutRerun(StatelessXmlReporter.java:221)
at org.apache.maven.plugin.surefire.report.StatelessXmlReporter.serializeTestClass(StatelessXmlReporter.java:211)
at org.apache.maven.plugin.surefire.report.StatelessXmlReporter.testSetCompleted(StatelessXmlReporter.java:161)
at org.apache.maven.plugin.surefire.report.StatelessXmlReporter.testSetCompleted(StatelessXmlReporter.java:85)
at org.apache.maven.plugin.surefire.report.TestSetRunListener.testSetCompleted(TestSetRunListener.java:193)
at org.apache.maven.plugin.surefire.booterclient.output.ForkClient$TestSetCompletedListener.handle(ForkClient.java:143)
```
The fix is to set a hardcoded value - `UndefinedException` as `error type` in case stack trace is empty.
<details>
<summary>checklist </summary>
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SUREFIRE) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `SUREFIRE-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean install` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the integration tests successfully (`mvn -Prun-its clean install`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "dr29bart (via GitHub)" <gi...@apache.org>.
dr29bart commented on PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#issuecomment-2161484463
> For me, this is fine now. Are you OK as well?
yes, I am OK to merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "dr29bart (via GitHub)" <gi...@apache.org>.
dr29bart commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1628020762
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -451,7 +451,9 @@ private static void getTestProblems(
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
- ppw.addAttribute("type", new StringTokenizer(stackTrace).nextToken());
+ ppw.addAttribute(
+ "type",
+ isBlank(stackTrace) ? "UndefinedException" : new StringTokenizer(stackTrace).nextToken());
Review Comment:
> I can remove the requried in the schema and bump to 3.1.0.
I like this idea. it is the easiest option to implement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "dr29bart (via GitHub)" <gi...@apache.org>.
dr29bart commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1449170752
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -451,7 +451,9 @@ private static void getTestProblems(
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
- ppw.addAttribute("type", new StringTokenizer(stackTrace).nextToken());
+ ppw.addAttribute(
+ "type",
+ isBlank(stackTrace) ? "UndefinedException" : new StringTokenizer(stackTrace).nextToken());
Review Comment:
Do you mean `indexOf` approach or `to use value as is`?
I didn't want to leave a chance for `type` to be null. What non-null value can be used here?
The `stackTrace` StatelessXmlReporter#439 var may be null:
```
public String getStackTrace(boolean trimStackTrace) {
StackTraceWriter w = original.getStackTraceWriter();
return w == null ? null : (trimStackTrace ? w.writeTrimmedTraceToString() : w.writeTraceToString());
}
```
At the same time surefire's XSD requires `type`
<details>
<summary>XSD</summary>
```xml
<xs:element name="error" nillable="true" minOccurs="0" maxOccurs="1">
<xs:complexType>
<xs:simpleContent>
<xs:extension base="xs:string">
<xs:attribute name="message" type="xs:string"/>
<xs:attribute name="type" type="xs:string" use="required"/>
</xs:extension>
</xs:simpleContent>
</xs:complexType>
</xs:element>
```
</details>
The goal is to handle stack traces like:
```
// has message
junit.framework.ComparisonFailure:
Expected :fail at foo
Actual :faild at foo
// does not have message
java.lang.AssertionError
at org.junit.Assert.fail(Assert.java:87)
at org.junit.Assert.fail(Assert.java:96)
// or stack trace is missing (empty String)
```
Please, let me know if this alternative is more preferable:
<details>
<summary>Option A</summary>
```java
if (report.getStackTraceWriter() != null) {
//noinspection ThrowableResultOfMethodCallIgnored
SafeThrowable t = report.getStackTraceWriter().getThrowable();
if (t != null) {
int delimiter = StringUtils.indexOfAny(stackTrace, ":", "\t", "\n", "\r", "\f");
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", Objects.toString(type, "UndefinedException"));
}
}
```
</details>
<details>
<summary>Option B</summary>
```java
SafeThrowable t = report.getStackTraceWriter().getThrowable();
if (t != null) {
if (t.getMessage() != null) {
int delimiter = stackTrace.indexOf(":");
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
ppw.addAttribute(
"type",
isBlank(stackTrace) ? stackTrace : new StringTokenizer(stackTrace).nextToken());
}
}
```
</details>
<details>
<summary>Option C</summary>
```java
SafeThrowable t = report.getStackTraceWriter().getThrowable();
if (t != null) {
if (t.getMessage() != null) {
int delimiter = stackTrace.indexOf(":");
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
int delimiter = StringUtils.indexOfAny(stackTrace, "\t", "\n", "\r", "\f");
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
}
}
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1627988457
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -451,7 +451,9 @@ private static void getTestProblems(
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
- ppw.addAttribute("type", new StringTokenizer(stackTrace).nextToken());
+ ppw.addAttribute(
+ "type",
+ isBlank(stackTrace) ? "UndefinedException" : new StringTokenizer(stackTrace).nextToken());
Review Comment:
I looked at this again and I want to address for 3.3.0.
I have option D for you: Since this is an oversight and `strackTrace` can be truly null of `t.getMessage()` is `null`, we should not write non-sense or empty to `type`, but leave it out. Rather no output than bad output. My proposal: Since we know that `type` can be null, I can remove the `requried` in the schema and bump to 3.1.0.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1635380077
##########
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java:
##########
@@ -522,6 +522,23 @@ public void testSyncOnDeferredFile() throws Exception {
assertThat((boolean) getInternalState(out, "closed")).isTrue();
}
+ public void testReporterHandlesATestWithoutMessageAndWithEmptyStackTrace() {
+ StackTraceWriter stackTraceWriterOne = new DeserializedStacktraceWriter(null, null, "");
+
+ WrappedReportEntry testReport = new WrappedReportEntry(
+ new SimpleReportEntry(
+ NORMAL_RUN, 1L, getClass().getName(), null, "a test name", null, stackTraceWriterOne, 5),
+ ERROR,
+ 5,
+ null,
+ null);
+
+ StatelessXmlReporter reporter = new StatelessXmlReporter(
+ reportDir, null, false, 1, new HashMap<>(), XSD, "3.0", false, false, false, false);
Review Comment:
This needs to be updated to 3.0.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#issuecomment-2150903376
@dr29bart First PR: https://github.com/apache/maven-surefire/pull/745
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#issuecomment-2160165869
@dr29bart I took the liberty to change your PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#issuecomment-2153400458
@dr29bart FYI: https://github.com/apache/maven-surefire/pull/746
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1449098296
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -451,7 +451,9 @@ private static void getTestProblems(
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
- ppw.addAttribute("type", new StringTokenizer(stackTrace).nextToken());
+ ppw.addAttribute(
+ "type",
+ isBlank(stackTrace) ? "UndefinedException" : new StringTokenizer(stackTrace).nextToken());
Review Comment:
Why don't you reuse the approach from line 451?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #716: [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable
URL: https://github.com/apache/maven-surefire/pull/716
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#discussion_r1628075126
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##########
@@ -451,7 +451,9 @@ private static void getTestProblems(
String type = delimiter == -1 ? stackTrace : stackTrace.substring(0, delimiter);
ppw.addAttribute("type", type);
} else {
- ppw.addAttribute("type", new StringTokenizer(stackTrace).nextToken());
+ ppw.addAttribute(
+ "type",
+ isBlank(stackTrace) ? "UndefinedException" : new StringTokenizer(stackTrace).nextToken());
Review Comment:
Let me then prepare a PR with that change and you can build up on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SUREFIRE-2232] StatelessXmlReporter: handle failed test result without a throwable [maven-surefire]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #716:
URL: https://github.com/apache/maven-surefire/pull/716#issuecomment-2150904406
I need to refresh my schema foo with https://www.xfront.com/SchemaVersioning.html and https://www.xfront.com/Versioning.pdf
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org