matthiasblaesing commented on code in PR #8568:
URL: https://github.com/apache/netbeans/pull/8568#discussion_r3005229458
##########
java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitNodeOpener.java:
##########
@@ -186,18 +188,18 @@ public void openCallstackFrame(Node node, @NonNull String
frameInfo) {
FileObject file = UIJavaUtils.getFile(frameInfo, lineNumStorage,
locator);
//lineNumStorage -1 means no regexp for stacktrace was matched.
if (testfo != null && file == null &&
methodNode.getTestcase().getTrouble() != null && lineNumStorage[0] == -1) {
- //213935 we could not recognize the stack trace line and map
it to known file
+ //213935 we could not recognize the stack trace line and map it to
known file
//if it's a failure text, grab the testcase's own line from the
stack.
String[] st =
methodNode.getTestcase().getTrouble().getStackTrace();
- if ((st != null) && (st.length > 0)) {
- int index = st.length - 1;
- //213935 we need to find the testcase linenumber to jump
to.
- // and ignore the infrastructure stack lines in the process
- while (!testfo.equals(file) && index != -1) {
+ for (int index = 0; !testfo.equals(file) && index < st.length;
index++) {
+ String trimmed = JavaRegexpUtils.specialTrim(st[index]);
+ if
(trimmed.startsWith(JavaRegexpUtils.CALLSTACK_LINE_PREFIX_CATCH)
+ ||
trimmed.startsWith(JavaRegexpUtils.CALLSTACK_LINE_PREFIX)) {
file = UIJavaUtils.getFile(st[index], lineNumStorage,
locator);
- index -= 1;
+ break;
}
}
+
}
Review Comment:
I see problems here:
- `UIJavaUtils.getFile` can yield `null` and while that is a bogus value, it
would stop the iteration.
- before the change there was a null guard, did you check, that
`getStackTrace` is always non null?
I also think that this method already does "the right thing". At least my
manual tests worked as expected (with this reverted I'm still taken to the
testcase, not the deeper assert in the same class/file).
The stacktrace starts with the "deepest" method and ends at the most top
level method. It is scanned from last to first index and stops at the first
method, for which the file matches the expected file. So it will match the
testcase method, not some deeply nested assert.
I remember, that I asked that this should unified, but I think ant and maven
drifted away from each other, so lets keep this as it was.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
