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

Remove global slim timeout during testing #660

Merged
merged 7 commits into from Mar 10, 2015
Merged

Conversation

six42
Copy link
Contributor

@six42 six42 commented Mar 1, 2015

This fixes #657 #656 #653
also fixes a bug with multiple arguments in SLIM_FLAGS.

I started preparing automatic test cases. They will follow shortly.

Before everything parsed in SLIM_FLAGS was treated as one additional
argument to the SlimService.
Now SLIM_FLAGS is split after each white-space character into distinct
parameters.
Fixes the problem that long running tests are stopped after slim.timeout
seconds.
New class for testing timeout in slim test acceptance test suite.
@amolenaar
Copy link
Collaborator

I created a few acceptance tests in the timeout branch on my fork. That might be useful. See #662.

@amolenaar
Copy link
Collaborator

Are you limiting the slim.timeout property to only be used while loading the test system?
I would expect it to be used when executing tests as well.

Both suites are marked as "skip" at the moment.
The slim.timeout flag doesn't works as assumed.
The suite for startup runs nice in interactive mode but fails when run
from ant. Input to fix this welcome.
@six42
Copy link
Contributor Author

six42 commented Mar 3, 2015

I expected this as well. But testing shows it is not the case. There is a workaround using SLIM_FLAGS. But also then the timeout doesn't seems to be useful as every testcase will fail afterwards.
See HowtoEnforceA_TimelimitPerTestCommand

I added the usage of slim.timeout while waiting for the SlimHeader message. The remaing usage of slim.timeout is unchanged.

Conflicts:
	src/fitnesse/testsystems/slim/SlimCommandRunningClient.java
Move the suite under SuiteSlimTests and removed includes which are not
required in this location
@six42
Copy link
Contributor Author

six42 commented Mar 7, 2015

Fixed the merge conflict to allow automatic merge.

String slimFlags = getVariable("slim.flags");
if (slimFlags == null) {
slimFlags = getVariable(SLIM_FLAGS);
}
return slimFlags == null ? "" : slimFlags;
return slimFlags == null ? new String[] {} : slimFlags.trim().split("\\s");
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a smarter parser in ClientBuilder.parseCommandLine(). That will take into account quoted values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint. Changed it.

Reuse existing method parseCommandLine to parse slim.flags. Supports
parameters in quotes.
@amolenaar amolenaar merged commit 102b835 into unclebob:master Mar 10, 2015
amolenaar added a commit that referenced this pull request Mar 10, 2015
Remove global slim timeout during testing
@amolenaar amolenaar added this to the Next release milestone Mar 20, 2015
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.

slim.timeout default value
2 participants