added error message for slim failure to output

[alexkogon]

No description provided.

[amolenaar]

I'm not sure, this solution adds a somewhat unpredictable chunk of data to the exception message. It's dependent on when the output of the SUT is flushed.

[alexkogon]

Hi,

I would certainly argue it is much better than a generic hard coded message which is very difficult to get any value from; the only way I could figure out why it wasn't working was to checkout fitnesse from git, set it up in my IDE (including needing to get ant and ivy working), connect a remote debugging session, then dig through multithreaded code to find this message. After hours spent doing that, figuring out the fix was fairly trivial.

It also is confusing as anyone with any problem gets the same error message, such as the open issue in the project which I commented on but which had nothing to do with our problem.

If you prefer a hard coded message that provides little value to one that tells you what happened when you run the command (though may run off the end of the view) that is up to you; personally I would also like to include how the command was run to produce the error and display it on multiple lines, but I didn't want to get into whatever was required to do so.

Either way I was able to identify my error and correct it, but it would have been much easier and nicer had I seen the actual message returned from the OS instead of a hard coded string.

[amolenaar]

I agree that the current situation can be improved. I do not prefer the hard coded message over a more to the point information.

I overlooked that the command runner is reading output per line. In that light it makes more sense to provide the last line of output. Wouldn't it be better to provide all the stderr content logged? Or maybe provide a link to the execution log? The output should be in there as well.

[alexkogon]

Hi,

You are right that it is logged (somewhere), that is in the code but I'm not sure where the logging is going--I just thought having a nice output in the web page showing the failure was handy.

Ideally I think we should output the Java command issued, and the error message output, but then it would need to be displayed on multiple lines; I tried that but the UI strips out the new lines and displays it all on one line, I suppose I could dig through the code more to figure that out but I thought this would be a good first step...if you are familiar with the code maybe you know where it is offhand, I'm quite new to Fitnesse I just had to do some hacking to figure out where our problem was coming from.

I'm happy to discuss a best solution, I'm guessing from your name you are probably in my time zone (and not so far from Amsterdam here as well :) )

[amolenaar]

An informative (multi-line) error message would be the most useful I suppose. For the short term I can merge this PR, while cooking up a more complete solution. I want to make a new release soon.

(near Amsterdam indeed: Hilversum, at XebiaLabs)

[six42]

If I get this error I press the "Errors Occurred" button.

The screen which comes up then shows the full error message and the command which was used to start the slim client. So all the information you are looking for.
Was this not working in your case?

You can quickly produce the error with the below statements on a test page

!define TEST_SYSTEM {SLIM}
!define slim.timeout {1}
!define TEST_RUNNER {something.that.does.not.exists}

see also:
http://fitnesse.org/FitNesse.SuiteAcceptanceTests.SuiteSlimTests.TestSystemSlimSuite.ErrorSituationsDuringSlimServerStartup.B_TimeoutAsSlimServerDiesBeforeConnecting

[amolenaar]

Indeed, but this message leaves the user with the question what to next. Either it should provide a call to action ("check the execution log for details") or some information pointing to the cause, which is what this patch provides.

[six42]

I like your idea to extend the error message with the text:
"check the execution log for details".
I would also propose to change the text on the button to "Execution Log…". I have the feeling that new users are not aware that this is a button as it looks like a Status Indicator. Think it is fine to change the colour of the button based on the test result but the text should not change.

Am 29. Juli 2015 11:22:50 MESZ, schrieb Arjan Molenaar notifications@github.com:

Indeed, but this message leaves the user with the question what to
next. Either it should provide a call to action ("check the execution
log for details") or some information pointing to the cause, which is
what this patch provides.

---

Reply to this email directly or view it on GitHub:
#772 (comment)

[alexkogon]

Hi Arjen,

My problem was that the underlying Java process couldn't launch because the classpath wasn't set properly when the Java process was spawned. This is a similar problem to issue #726 which I commented on, though it was happening for a different reason. I added this code so that when the Java process won't launch you see the output in the UI; I wasn't really thinking that the Java process could also just die for whatever reason, and this would also be output.

I think in the instance where the underlying process is not working (granted, the reason was I had been hacking on the codebase, but this is happening to others too) it is great to have a multi-line output that shows the java command line run, and the error message. In the case where it just died after running for a while, the output is probably still helpful but the java command line may not be that interesting.

Perhaps we could make FitNesse smart enough to know that the server died on startup and have two different messages if so?

Let me know if you want me to help on this, I'm not so familiar with the browser code but could probably figure it out or at least collaborate with someone to finalize it. From our point of view the problem was that the java process was dying when we launched it, I figured out why so it is not such a pressing need but having the better error message would have saved us a lot of headaches.

[amolenaar]

I put your comments in a follow-up ticket #782. That way the discussion doesn't get lost. I think it's a good thing to improve user feedback in the SUT execution area for the next release.

[alexkogon]

Sounds good. Let's catch up later on how you want to implement and whether you want my help; like I said I wanted to output the java command run and the error message, but when I inserted new lines in the string it still displayed just a single line. That part was pretty easy (since I already found where to do it), I'd have to spend some time searching for the client side so if you know where that is off hand maybe we can collaborate, just let me know.