Some exceptions don't get indicator/link in tables

[stanio]

Spun off #150:

Not sure how it might be related, but regular exception thrown from the input method doesn't get an indicator/link in the decision table - just the stack trace displayed prior that, however the custom message exception gets shown in the table. Seems like every exception which gets an indicator where it has happened, is doubly counted.

I've observed the same behavior of not showing indicator for regular exceptions, but for "message exceptions", with script tables, also - for void methods or rows of regular function calls which don't perform symbol assignment, for example.

May be (just may be) solving this would make more obvious which of the two exception increasing statements mentioned in #150 further, is superfluous.

[amolenaar]

Confirmed

[amolenaar]

This is fixed with the merge of #217

[stanio]

While the #217 changes bring some nice innovations (I like the exceptions being reported immediately after the corresponding fixture row), currently I find the following flaws:

1. Because all of the cells in a failed row (decision or script) are marked as class="error", the Failure Navigator widget reports too many errors. May be for the purpose of styling the row element itself could be marked as failure, and then the widget could count/navigate only the error cells;
2. If there are multiple (non-message) exceptions for a single (decision) row, only the last one appears visible after expanding. I can see the other exceptions (<tr class="exception-detail">) are also listed in the content but their initial class="closed-detail" doesn't seem triggered by the expansion. Further, because of the lack of hyper-linking (or other type of association) it is hard to say which exception detail is related to which cell;
3. No stack traces available. Is there an alternative solution without having to surround every fixture method using try-catch?

Are the given issues know (I'll search the issue list later) and is there a plan to address them further?

[amolenaar]

I'll check out issue 2. Technically there should be only one message.

Regarding issue 3: I think we missed something in the "big slim test system refactoring".

Issue 1: I think we should also have a look at failed scenarios here.

[amolenaar]

I fixed issue 3.

[stanio]

Yep, I've tried it also - the stack traces are there. Thank you very much. :-)

[amolenaar]

I think issue 2 is not appearing. There can be only one error registered per row, which will be displayed.

[stanio]

I've fixed 2 for me like:

diff -r 02368025245e src/fitnesse/resources/javascript/fitnesse.js
--- a/src/fitnesse/resources/javascript/fitnesse.js Wed Oct 31 23:28:56 2012 +0200
+++ b/src/fitnesse/resources/javascript/fitnesse.js Wed Apr 10 21:01:56 2013 +0300
@@ -14,17 +14,17 @@
   $.get(url);
   return false;
 }

 /**
  *  Scenario's and Exceptions (after test execution)
  */
 $(document).on("click", "article tr.scenario td, article tr.exception td", function () {
-   $(this).parent().toggleClass('closed').next().toggleClass("closed-detail");
+   $(this).parent().toggleClass('closed').nextUntil(":not(.exception-detail)").toggleClass("closed-detail");
 });

 /**
  * Collapsible section
  */
 $(document)
    .on("touchstart click", "article .collapsible > p.title", function () {
        $(this).parent().toggleClass('closed');

I still miss the old hyper-linking and I think the current solution could be combined with the old behavior, but we could address this in a separate enhancement request.

One can see all exceptions without this patch by turning off author styles in the browser (e.g. View -> Page Style -> No Style, in Firefox), or just explore the DOM with whatever tool available.

[stanio]

To elaborate on issue 2:

1. Create a Slim test page with the following decision table:

   |some exceptions|
   |foo |bar |baz? |
   |-   |    |/    |
   |    |*   |     |
   

2. Use the following fixture class:

   public class SomeExceptions {
   
       public void setFoo(String foo) {
           throw new IllegalArgumentException("sample foo error");
       }
   
       public void setBar(String bar) {
           throw new UnsupportedOperationException("artificial bar error");
       }
   
       public String baz() {
           throw new IllegalStateException("dummy baz error");
       }
   
   }

[amolenaar]

Hi, I think I nailed the error navigator as well.

[amolenaar]

I thought... Your comments just popped up. I'll check. Fix looks nice, though :)

[stanio]

Fix looks nice, though :)

Just saw the comment on the click handler: "Scenario's and Exceptions" - the fix seems to miss/break the scenarios. The changed selector should be extended to match those as well.

[stanio]

Updated patch for issue 2:

diff -r 02368025245e src/fitnesse/resources/javascript/fitnesse.js
--- a/src/fitnesse/resources/javascript/fitnesse.js Wed Oct 31 23:28:56 2012 +0200
+++ b/src/fitnesse/resources/javascript/fitnesse.js Wed Apr 10 21:01:56 2013 +0300
@@ -14,17 +14,17 @@
   $.get(url);
   return false;
 }

 /**
  *  Scenario's and Exceptions (after test execution)
  */
 $(document).on("click", "article tr.scenario td, article tr.exception td", function () {
-   $(this).parent().toggleClass('closed').next().toggleClass("closed-detail");
+   $(this).parent().toggleClass('closed').nextUntil(":not(.exception-detail, .scenario-detail)").toggleClass("closed-detail");
 });

 /**
  * Collapsible section
  */
 $(document)
    .on("touchstart click", "article .collapsible > p.title", function () {
        $(this).parent().toggleClass('closed');

[amolenaar]

Works like a charm. But wouldn't it be nicer if all exceptions are put in the same table row?

[amolenaar]

I pushed it in b78d5f3. I used to page FitNesse.UserGuide.SliM.DecisionTable to place the example on (need some browser testing to test it).

[stanio]

But wouldn't it be nicer if all exceptions are put in the same table row?

Yes, I think it is perfectly valid solution, too.

[stanio]

Hi, I think I nailed the error navigator as well.

Using Edge build #325 which includes 2122fbe, I still get confusing results, but I think is not really possible to match exact wrong/exceptions counts with the test output error indicators count.

Looking at it some more I think the "Failure Navigator" could really count/navigate only through table rows which contain error(s), without going through individual error cells. But that would be a change others should accept/comment on, also.

[mgaertne]

I am not sure what should happen with a Table table where each cell may get different behavior.

Best Markus

Dipl.-Inform. Markus Gärtner
Author of ATDD by Example - A Practical Guide to Acceptance
Test-Driven Development
Agile Team Academy, September, Amsterdam http://www.agileteamacademy.com/

http://www.shino.de/blog
http://www.mgaertne.de
http://www.it-agile.de
Twitter: @mgaertne

On 13.04.2013, at 08:58, Stanimir Stamenkov notifications@github.com wrote:

Hi, I think I nailed the error navigator as well.

Using Edge build #325 which includes 2122fbe, I still get confusing results, but I think is not really possible to match exact wrong/exceptions counts with the test output error indicators count.

Looking at it some more I think the "Failure Navigator" could really count/navigate only through table rows which contain error(s), without going through individual error cells. But that would be a change others should accept/comment on, also.

—
Reply to this email directly or view it on GitHub.

[stanio]

I am not sure what should happen with a Table table where each cell may get different behavior.

If you happen to comment on my suggestion the "Failure Navigator" should only care about going through table rows, my rationale is:

Even currently it doesn't become obvious which error cell is focused by the navigator (at least with the bootstrap theme). Navigating through number of error cells in a single row doesn't have any visual indication (apart from the dubious error number in the navigator) - no scrolling, and after all I think the biggest strength and purpose of the "Failure Navigator" is to find failures in a long test run easily. That is, once you've been taken to the error row - you could "parse" the rest on your own. But that's just my opinion. :-)

[stanio]

In any case, the original issue is fixed. Anything else related to the "Failure Navigator" should be addressed separately. Sorry for keeping this so long.