[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17802226#comment-17802226
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1440602428
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
Fix: https://github.com/apache/maven-surefire/pull/709
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4, 3.2.5
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17802115#comment-17802115
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1440318628
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
Vote canceled.
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
Vote canceled and repo dropped.
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17802091#comment-17802091
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1440247439
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
Thanks for the confirmation. I will cancel the vote today, create an
improvement request to change the schema in the next major to a complex element
with `stackTrace` to align with the rest and another one to fix this
regression. I will ping you as soon as I have something to test.
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17802035#comment-17802035
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
marcphilipp commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1440135518
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
Yes, that's right.
## Before (with 3.2.3 and earlier)
```xml
```
## After this change
```xml
```
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801874#comment-17801874
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1439670377
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
I think I understand what you mean. Let me rephrase and I guess you are
right here, the schema is poorly designed. According to
https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
both `failure` and `error` aren't complex elements, thus do *not* contain
`stackTrace`. Their element body contains the strack trace? Is that the case,
if so I will cancel the vote and work on a fix and let you know.
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801873#comment-17801873
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1439670377
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
I think I understand what you mean. Let me rephrase and I guess you are
right here, the schema is poorly designed. According to
https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
both `failure` and `error` aren't complex elements, thus do *not* contain
`stackTrace`. Their element body contains the strack trace?
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801871#comment-17801871
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
marcphilipp commented on code in PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#discussion_r1439664959
##
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java:
##
@@ -456,21 +456,13 @@ private static void getTestProblems(
}
}
-boolean hasNestedElements = createOutErrElementsInside & stackTrace !=
null;
-
-if (stackTrace != null) {
-if (hasNestedElements) {
-ppw.startElement("stackTrace");
+if (enableNestedOutErrElements) {
+ppw.startElement("stackTrace");
+if (stackTrace != null) {
+extraEscapeElementValue(stackTrace, outputStreamWriter, ppw,
fw);
}
+ppw.endElement();
-extraEscapeElementValue(stackTrace, outputStreamWriter, ppw, fw);
Review Comment:
This still needs to be called if `stackTrace != null` and
`!enableNestedOutErrElements`. Otherwise, `error` and `failure` elements won't
contain the stack trace as their content.
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17799920#comment-17799920
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
asfgit merged PR #702:
URL: https://github.com/apache/maven-surefire/pull/702
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17799806#comment-17799806
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
michael-o commented on PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#issuecomment-1867589786
> @michael-o, actually, I am a bit out of context here and not 100% sure
what that code does and currently am busy otherwise, i.e. I am just looking at
the code here on the website, which IMO is **not** a good way to conduct a
proper review. You have to check out the code and see what it does, inspect/run
tests covering the code etc.
>
> It looks as if in case of reported test errors or failures, it adds
something like an "error" or "failure" tag to the XML and then, if any stack
trace exists, also a "stackTrace" element and "system-out"/"system-err".
>
> Am I seeing correctly, that you just refactored the code with no
functional changes, only reflecting the nesting structure of the XML output in
the logical structure (`if` block) of the source code? If so, it looks good to
me.
Just refactoring and abiding to the XML schemas:
https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
and
https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (SUREFIRE-2224) StatelessXmlReporter#getTestProblems() does not properly reflect report schema structure
[
https://issues.apache.org/jira/browse/SUREFIRE-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17799624#comment-17799624
]
ASF GitHub Bot commented on SUREFIRE-2224:
--
kriegaex commented on PR #702:
URL: https://github.com/apache/maven-surefire/pull/702#issuecomment-1867103417
@michael-o, actually, I am a bit out of context here and not 100% sure what
that code does and currently am busy otherwise, i.e. I am just looking at the
code here on the website, which IMO is **not** a good way to conduct a proper
review. You have to check out the code and see what it does, inspect/run tests
covering the code etc.
It looks as if in case of reported test errors or failures, it adds
something like an "error" or "failure" tag to the XML and then, if any stack
trace exists, also a "stackTrace" element and "system-out"/"system-err".
Am I seeing correctly, that you just refactored the code with no functional
changes, only reflecting the nesting structure of the XML output in the logical
structure (`if` block) of the source code? If so, it looks good to me.
> StatelessXmlReporter#getTestProblems() does not properly reflect report
> schema structure
>
>
> Key: SUREFIRE-2224
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2224
> Project: Maven Surefire
> Issue Type: Bug
> Components: xml generation
>Affects Versions: 3.2.3
>Reporter: Michael Osipov
>Assignee: Michael Osipov
>Priority: Major
> Fix For: 3.2.4
>
>
> According to
> https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd
> and
> https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report.xsd
> not all nested elements contain {{stackTrace}}, {{system-out}} and
> {{system-err}} elements, but the code makes it conditional whether there is a
> stack trace or not. This does not comply with the schema definition. Either
> the nested element is simple content or complex. In the latter {{stackTrace}}
> is mandatory, even if there is no strack trace according to the schema. The
> system streams are optional.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
