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

Try to gracefully terminate the running test in case of an exception #31

Merged
merged 3 commits into from Dec 7, 2016

Conversation

Projects
None yet
2 participants
@rudopeters
Copy link
Contributor

rudopeters commented Dec 4, 2016

This change will stop the running test when an exception occurs, for example when the test timeout is exceeded, or when the user aborts the Jenkins job. This is especially useful in case the Fitnesse instance was already running. Will probably fix #12377.

@@ -68,6 +71,11 @@ public boolean execute(Launcher launcher,FilePath workspace, Run<?,?> build) thr
throw (InterruptedException) t;
return false;
} finally {
try {
killTest(getFitnessePage(build, false));

This comment has been minimized.

@lessonz

lessonz Dec 5, 2016

Member

Why not put this in the catch instead of relying on setting fitnesseTestId?

This comment has been minimized.

@rudopeters

rudopeters Dec 6, 2016

Author Contributor

I'm sorry, trying to understand what you are saying... do you mean I should put
try { killTest(getFitnessePage(build, false)); (...)
in the catch block instead of in the finally block?

This comment has been minimized.

@lessonz

lessonz Dec 6, 2016

Member

Right, if I'm understanding the code, you're calling killTest every time but relying on a member variable to determine whether or not it should do something. Couldn't you just call killTest in the catch and not have to rely on the member variable. Feel free to correct me. I may be missing something.

This comment has been minimized.

@rudopeters

rudopeters Dec 6, 2016

Author Contributor

I think I see what you mean. I have changed the code so it will only be executed once the catch is executed.

rudopeters added some commits Dec 6, 2016

@lessonz lessonz merged commit 63228f1 into jenkinsci:master Dec 7, 2016

1 check passed

Jenkins This pull request looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.