matthiasblaesing commented on code in PR #8568:
URL: https://github.com/apache/netbeans/pull/8568#discussion_r2233881007


##########
java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitNodeOpener.java:
##########
@@ -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();

Review Comment:
   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.



##########
java/junit.ant.ui/src/org/netbeans/modules/junit/ant/ui/AntJUnitNodeOpener.java:
##########
@@ -157,29 +160,42 @@ public void openCallstackFrame(Node node, String 
frameInfo) {
         }
         // Method node might belong to an inner class
         FileObject testfo = methodNode.getTestcase().getClassFileObject(true);
+        String fqMethodName = methodNode.getTestcase().getClassName() + '.' + 
methodNode.getTestcase().getName();
        if(testfo == null) {
            return;
        }
         final int[] lineNumStorage = new int[1];
         FileObject file = UIJavaUtils.getFile(frameInfo, lineNumStorage, 
locator);
         //lineNumStorage -1 means no regexp for stacktrace was matched.
         if ((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.
-            boolean methodNodeParentOfStackTraceNode = false;
             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.
+                int index = 0;//st.length - 1;
+                //Jump to the first line matching the fully qualified test 
method name.
                 // and ignore the infrastructure stack lines in the process
-                while (!testfo.equals(file) && index != -1 && 
!methodNodeParentOfStackTraceNode) {
-                    file = UIJavaUtils.getFile(st[index], lineNumStorage, 
locator);
-                    index = index - 1;
-                    // if frameInfo.isEmpty() == true, user clicked on a 
failed method node. 
-                    // Try to find if the stack trace node is relevant to the 
method node
-                    if(file != null && frameInfo.isEmpty()) {
-                        methodNodeParentOfStackTraceNode = 
FileUtil.isParentOf(testfo.getParent(), file);
+                while (index < st.length) {
+                    if (st[index].contains(fqMethodName)) {
+                        file = UIJavaUtils.getFile(st[index], lineNumStorage, 
locator);
+                        break;
                     }
+                    index++;
+                }
+                // if not found, return top line of stack trace.
+                if (index == st.length) {
+                    index=0;
+                    while(true) {

Review Comment:
   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:
   
   
   ```suggestion
                       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.



##########
java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitNodeOpener.java:
##########
@@ -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();
             String[] st = 
methodNode.getTestcase().getTrouble().getStackTrace();
             if ((st != null) && (st.length > 0)) {
-                int index = st.length - 1;
+                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++;

Review Comment:
   ```suggestion
                   // 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;
                       }
                   }
   ```



##########
java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitNodeOpener.java:
##########
@@ -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();
             String[] st = 
methodNode.getTestcase().getTrouble().getStackTrace();
             if ((st != null) && (st.length > 0)) {
-                int index = st.length - 1;
+                int index = 0;//st.length - 1;

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



##########
java/junit.ant.ui/src/org/netbeans/modules/junit/ant/ui/AntJUnitNodeOpener.java:
##########
@@ -157,29 +160,42 @@ public void openCallstackFrame(Node node, String 
frameInfo) {
         }
         // Method node might belong to an inner class
         FileObject testfo = methodNode.getTestcase().getClassFileObject(true);
+        String fqMethodName = methodNode.getTestcase().getClassName() + '.' + 
methodNode.getTestcase().getName();
        if(testfo == null) {
            return;
        }
         final int[] lineNumStorage = new int[1];
         FileObject file = UIJavaUtils.getFile(frameInfo, lineNumStorage, 
locator);
         //lineNumStorage -1 means no regexp for stacktrace was matched.
         if ((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.
-            boolean methodNodeParentOfStackTraceNode = false;
             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.
+                int index = 0;//st.length - 1;
+                //Jump to the first line matching the fully qualified test 
method name.
                 // and ignore the infrastructure stack lines in the process
-                while (!testfo.equals(file) && index != -1 && 
!methodNodeParentOfStackTraceNode) {
-                    file = UIJavaUtils.getFile(st[index], lineNumStorage, 
locator);
-                    index = index - 1;
-                    // if frameInfo.isEmpty() == true, user clicked on a 
failed method node. 
-                    // Try to find if the stack trace node is relevant to the 
method node
-                    if(file != null && frameInfo.isEmpty()) {
-                        methodNodeParentOfStackTraceNode = 
FileUtil.isParentOf(testfo.getParent(), file);
+                while (index < st.length) {
+                    if (st[index].contains(fqMethodName)) {
+                        file = UIJavaUtils.getFile(st[index], lineNumStorage, 
locator);
+                        break;
                     }
+                    index++;
+                }
+                // if not found, return top line of stack trace.
+                if (index == st.length) {
+                    index=0;
+                    while(true) {
+                        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);
+                            break;
+                        }
+                    }
+                }
+                // if that fails, return the test file object.
+                if (file == null) {
+
+                    file = testfo;
                 }

Review Comment:
   You have `methodNode` availble, I suggest to fallback to `openTestMethod`.



-- 
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: notifications-unsubscr...@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to