Skip to content
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

Watch closer what SLIM is doing #1500

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

Watch closer what SLIM is doing #1500

wants to merge 5 commits into from

Conversation

six42
Copy link
Contributor

@six42 six42 commented Apr 1, 2024

This is an alternative implementation to #1469 from @pvbemmelen62
As it is fully implemented in the Slim Server it will work for all Slim ports and not only for the Java Slim Client as the other mentioned PR.

It adds a new feature to see the instructions SLIM is creating step by step and also see how a SLIM table changes with each execution.
To achieve this result the Slim Test System needed a change to suport processing of SINGLE instructions.
The existing SLIM implementation always sends a full table to the SLIM client for execution, which I call BULK mode. In the past there may have been performence concernces for the network communication to do it in this bulk way. Nowdays this is not an issue.
Modern protocols like the Language Server Protocol or Debug Adapter Protocol work also in a step by step approach to get the best interactive experience. Nevertheless this new SINGLE mode is not active by default. Default is still the BULK mode. Only when more interaction is requested the SLIM test system will switch to this mode.

Usage Guide - From the updated Wiki page

When a SLIM test is run the the webpage will update each time a table has been fully processed.
If you want to get quicker an undate you have to set the property: ''slim.show''

At the top of the test page you will then see 2 additional outputs

  1. A list of the last 10 instructions the SLIM server is sending to the SLIM Client and the results of the same.
  2. The current table and how its cells are updated after each instruction executed.

In addition you can use the following two properties to customize this further:

  • slim.show.size - default 10 - Number of instructions to show

  • slim.show.sleep - default 0 - Waits for X milliseconds between each instruction call. This is for making nice videos of a test run. Otherwise I don't think there is a usecase for slowing down the execution of your test

Those properties can be either provided by a wiki page, on the command line (e.g. ''-Dslim.port=9000'') or in the plugins.properties file.

Watch the video:

Fitnesse_StepByStep_final.mp4

Sample Invocation:
http://localhost:80/FitNesse.UserGuide.TwoMinuteExample?test&slim.show&slim.show.sleep=90&slim.show.size=15

…ts after each instruction execution

Two new parameters can be given on the command line to see what fitnesse is currently doing
1)
A) slim.sbys=SHOWTABLE
show the html table after each executed instruction
B) slim.sbys=SHOWINSTRUCTIONS
show the instruction after each executed instruction
C) slim.sbys=TRUE
instead of sending a full tabke to the slim client send just one instruction each time
2) only if 1 is given, will delay the execution by 0.5 seconds just useful for showing the beaviour
slim.sbys.sleep=SLEEP

Examples:
http://localhost/FitNesse.SuiteAcceptanceTests.SuiteSlimTests.TestComparators?test&slim.sbys=SHOWINSTRUCTIONS&slim.sbys.sleep=SLEEP
http://localhost/FitNesse.SuiteAcceptanceTests.SuiteSlimTests.TestComparators?test&slim.sbys=SHOWTABLE&slim.sbys.sleep=SLEEP
Added documentation
all code for the display is now in the SuiteHtmlFormatter and not in the SlimTestSystem
@pvbemmelen62
Copy link

Hi six42 ,

I see a couple of drawbacks in having the slim instructions shown in the run log page itself:

  1. the java code is complex, compared to when showing it in a separate window.
  2. the slim instructions are shown at the top, but the run log page grows at the bottom: you can't see both at the same time.

And you don't see the currently running instruction , which is a big minus.

Ad 1) The complexity is added in the current code. In #1469 , which uses a separate window, any added complexity is in the code for the extra window. It doesn't add complexity to existing code.

Okay... #1469 uses a Swing window. @fhoeben likes to have a browser window showing the slim instructions. Maybe I will release the "browser version" that I currently have, even if it is not quite usable because it hardcodes the browser port. That way others can take a look at it, and maybe help in adding "a separate option" for specifying the browser port .

@six42
Copy link
Contributor Author

six42 commented Apr 17, 2024

Hi @pvbemmelen62 Paul,
thanks for taking the time and looking at the PR.
Following the fitnesse architecture might look complex. Having the reporting happening in the reporting engine I see as an advantage. It should not be done by the SUT. It has the advantage that this PR will work with any language SLIM supports.
And it add the feature of the updating tables in real time.

Just showing the finished instructions is a limitation of the current interface design and could be changed by addding one more method to the Interface: TestSystemListener.

If a test is bigger than one page length my browser is not scrolling to the bottom automatically. Is yours doing this?
So I anyway never see the current test output with Fitnesse.
Showing the new data at the top was a choise to ensure the instructions are always visible. For me it feels better than what was available so far.
It could also be appended to the bottom but as said than you are busy with scrolling all the time and can less focus on what the test is doing.

Removed the limitation that the instructions are only show after the execution.
Now they are first shown with as "in progress" and after execution the actual result is added.
@six42
Copy link
Contributor Author

six42 commented Jul 1, 2024

Removed the limitation that the instructions are only show after the execution.
Now they are first shown as "in progress" and after execution the actual result is added.

FitNesse.sbys.inProgress.2024-07-01.015051.mp4

@fhoeben
Copy link
Collaborator

fhoeben commented Aug 16, 2024

@six42 I like the idea of seeing progress as its happening very much. In the past it always bugged me to have to split up a long script table in multiple to have some overview of where the execution was.

Some minor notes. I think we should require a true value for the slim.show property. That makes it easier to configure it always and just changing the value to enable/disable.

Have you checked whether the HTML output generated to file via a jUnit run is changed. I don't expect it, but just wondering.

Furthermore some unit tests and an update of the release notes would have been nice...

@six42
Copy link
Contributor Author

six42 commented Aug 16, 2024 via email

Copy link
Collaborator

@fhoeben fhoeben left a comment

Choose a reason for hiding this comment

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

I general take a look at the formatting/whitespace of the new code. I believe there are a lot of places where there is no space between if and the opening parenthesis or between ){.


import static fitnesse.testsystems.ExecutionResult.getExecutionResult;

public class SuiteHtmlFormatter extends InteractiveFormatter implements Closeable {
public class SuiteHtmlFormatter extends InteractiveFormatter implements Closeable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public class SuiteHtmlFormatter extends InteractiveFormatter implements Closeable {
public class SuiteHtmlFormatter extends InteractiveFormatter implements Closeable {

String sbys = getPage().getVariable(SLIM_SHOW);
testSystemListenerShowHtmlTable = (sbys != null);
testSystemListenerShowInstructions = 0;
if(testSystemListenerShowHtmlTable){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if(testSystemListenerShowHtmlTable){
if (testSystemListenerShowHtmlTable) {

Missing whitespaces like this in many places

}
}
//Fill the list with empty lines to reserve the space on the screen
this.instructionsHtmlText = new LinkedList<String>(Collections.nCopies(testSystemListenerShowInstructions,""));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
this.instructionsHtmlText = new LinkedList<String>(Collections.nCopies(testSystemListenerShowInstructions,""));
this.instructionsHtmlText = new LinkedList<String>(Collections.nCopies(testSystemListenerShowInstructions, ""));

@Override
public void testAssertionVerified(Assertion assertion, TestResult testResult) {
String resultString = testResult != null ? testResult.toString() : "VOID";
showAssertionResult(assertion,resultString, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
showAssertionResult(assertion,resultString, true);
showAssertionResult(assertion, resultString, true);

}

@Override
public void testAssertionVerified(Assertion assertion, TestResult testResult) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a unit test checking that testAssertionVerified, testExceptionOccurred and testAssertionWillBeExecuted all call showAssertionResult() (with the right parameters) would be good already. The actual HTML and JavaScript created don't have to be tested yet

}
}

private boolean evaluateAssertion(Object InstructionResult, boolean IgnoreTestTable, SlimAssertion a, String key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great improvement to have this in a separate method. I think by making it protected the code using it should be a bit easier to test, and this might aid future changes.

Suggested change
private boolean evaluateAssertion(Object InstructionResult, boolean IgnoreTestTable, SlimAssertion a, String key)
protected boolean evaluateAssertion(Object instructionResult, boolean ignoreTestTable, SlimAssertion assertion, String key)

stopSuiteCalled = PageData.SUITE_SETUP_NAME.equals(testContext.getPageToTest().getName());
}
if (exceptionResult.isStopSuiteException()) {
//IgnoreTestTable = stopTestCalled = stopSuiteCalled = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
//IgnoreTestTable = stopTestCalled = stopSuiteCalled = true;

if (InstructionResult != null && InstructionResult instanceof String && ((String) InstructionResult).startsWith(EXCEPTION_TAG)) {
SlimExceptionResult exceptionResult = new SlimExceptionResult(key, (String) InstructionResult);
if (exceptionResult.isStopTestException()) {
//IgnoreTestTable = stopTestCalled = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
//IgnoreTestTable = stopTestCalled = true;

if (testSystemListenerSleep > 0)
Thread.sleep(testSystemListenerSleep);
} catch (Exception e){
String em = e.getMessage();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This exception is ignored. I assume it is never expected to happen. Shouldn't it be logged (so it shows up in the console where the wiki was started)?

String insertScript = JavascriptUtil.makeReplaceElementScript("step-by-step-Id2", html).html();
writeData(insertScript);
} catch (Exception e){
String html = e.getMessage();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This exception is ignored. I assume it is never expected to happen. Shouldn't it be logged (so it shows up in the console where the wiki was started)?

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.

3 participants