Skip to content

fixes evaluation order of stack trace #8568

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

homberghp
Copy link
Contributor

@homberghp homberghp commented Jun 7, 2025

This version works for ant projects. (Including netbeans projects).
This PR closes #8567

This implementation of finding file object+line number reverses the search by starting at the top of the stack and try to find the stack frame that is directly related to the test method.

It reverses the loop running from bottom to top into running from the top stack frame to the bottom one.
It effectively tries to find the first stack element that matches the testMethod involved instead of the first element that is NOT in some (testing) framework. For most tests this appears to be the logical choice, in particular in the non-optimal practice of having multiple assert or assert-like statements in one test method.

For NetBeans developers, in particular those working on refactoring related stuff and tests, it is more useful to point to the failing assert or verifyXXX call than in some method inside some method inside the test class or a superclass thereof that generalizes repetitive work in the tests.

In the original the goto source landed me in

@Override
protected void runTest() throws Throwable {
//the tests are unfortunatelly not 100% stable, try to recover by retrying:
Throwable exc = null;
for (int i = 0; i < RETRIES; i++) {
try {
super.runTest();
return;
} catch (Throwable t) {
if (exc == null) exc = t;
}
}
throw exc;
}

instead of say line 118 below, which is the more logical line to choose in most cases.

This PR solves the problem for both maven and ant projects in a similar way.

@homberghp homberghp changed the title solves issue8567 by reversing order of stack evaluation closes 8567, evaluation order of stack trace Jun 13, 2025
@homberghp homberghp changed the title closes 8567, evaluation order of stack trace fixes evaluation order of stack trace Jun 13, 2025
@homberghp
Copy link
Contributor Author

solves #8567

Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. The approach makes sense to me. I left a few inline comments. For individual commits: They can be separate, but don't need to be. For the summary of the commit I would suggest:

"JUnit: Fix 'jump to test' for failing unittests"

If I understand it correctly, The stacktrace jumping itself is not affected, it is just the jump, when a failing testcase is selected and a non-stacktrace line is selected and thus no "real" target can be determined.

// if not found, return top line of stack trace.
if (index == st.length) {
index=0;
while(true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is broken. The loop will not end, you are not incrementing index and even if, the target method might not be on the stack. So I suggest:

Suggested change
while(true) {
for(index=0; index < st.length; index++) {

As long as index is not modified outside the loop initilizer, this will not be an infite loop and still retain the early exist option.

Comment on lines +195 to 199
// if that fails, return the test file object.
if (file == null) {

file = testfo;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have methodNode availble, I suggest to fallback to openTestMethod.

@@ -188,14 +188,18 @@ public void openCallstackFrame(Node node, @NonNull String frameInfo) {
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
//if it's a failure text, grab the testcase's own line from the stack.
String fqMethodName= methodNode.getTestcase().getClassName()+ '.'+ methodNode.getTestcase().getName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work (at least in my tests it does not). The name of the testcase also holds the class name, but maybe that is already the problem and we should fix it at the root?!

This is the code I use to fix the nested test cases:

https://github.com/apache/netbeans/pull/8664/files#diff-20f21830cc72d9977dabeae930571d61083d0f61de40c3fd9637e5c9c69ec461R223-R239

Seems for ant this is not a problem as the report is extracted differently.

String[] st = methodNode.getTestcase().getTrouble().getStackTrace();
if ((st != null) && (st.length > 0)) {
int index = st.length - 1;
int index = 0;//st.length - 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the old code. If it is needed, version history is there.

Comment on lines +194 to +202
int index = 0;//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) {
file = UIJavaUtils.getFile(st[index], lineNumStorage, locator);
index = index - 1;
while (!testfo.equals(file) && index < st.length) {
if (st[index].contains(fqMethodName)) {
file = UIJavaUtils.getFile(st[index], lineNumStorage, locator);
break;
}
index++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int index = 0;//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) {
file = UIJavaUtils.getFile(st[index], lineNumStorage, locator);
index = index - 1;
while (!testfo.equals(file) && index < st.length) {
if (st[index].contains(fqMethodName)) {
file = UIJavaUtils.getFile(st[index], lineNumStorage, locator);
break;
}
index++;
// 213935 we need to find the testcase linenumber to jump to.
// and ignore the infrastructure stack lines in the process
for(int index = 0; !testfo.equals(file) && index < st.length; index++) {
if (st[index].contains(fqMethodName)) {
file = UIJavaUtils.getFile(st[index], lineNumStorage, locator);
break;
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

junit failed test jumps to wrong test line
2 participants