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

Prevent transient job state test failures from failing the build. #4510

Merged
merged 1 commit into from Aug 30, 2017

Conversation

Projects
None yet
3 participants
@jmchilton
Copy link
Member

commented Aug 29, 2017

A precondition to the real meat of the test is failing to be met sometimes in production - this failure is indicating a problem with the test and not with Galaxy. I previously tried to address this with #3988 but that didn't work. Now if the trainsient ok occurs the test will be skipped. Most of the time it won't skip and the rest of the test will execute and ensure there aren't regressions in behaviors related to cleaning up datasets after job completion. I'm placing the skips a couple different places so hopefully we can get a stack trace at some point - knowing where the code is when the job state is changing from running to ok will help puzzle out what is handing for the two minutes the job is running in the test framework.

Prevent transient job state test failures from failing the build.
A precondition to the real meat of the test is failing to be met sometimes in production - this failure is indicating a problem with the test and not with Galaxy. I previously tried to address this with #3988 but that didn't work. Now if the trainsient ok occurs the test will be skipped. Most of the time it won't skip and the rest of the test will execute and ensure there aren't regressions in behaviors related to cleaning up datasets after job completion. I'm placing the skips a couple different places so hopefully we can get a stack trace at somepoint - knowing where the code is when the job state is changing from running to ok will help puzzle out what is handing for the two minutes the job is running in the test framework.
@martenson

This comment has been minimized.

Copy link
Member

commented Aug 29, 2017

./lib/galaxy/datatypes/data.py:372:21: F841 local variable 'tmp_file_name' is assigned to but never used

I think this is the fallout of the security patch fallout fix

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2017

#4515 should fix the build problems? Can we merge this in meantime?

@dannon

This comment has been minimized.

Copy link
Member

commented Aug 29, 2017

So the test requires a job in the running state? Should we instead have a dummy tool that always takes, for example, 5 seconds of sleep() time or something?

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Aug 30, 2017

The tool it is using sleeps for 120 seconds - looking at the timing I think it is actually sleeping for 120 seconds also. I think that part is working as far as I can tell. Just something in the test is occasionally taking more than that - possibly something is blocking on the state of the job to change somehow?

@martenson martenson merged commit 2f9cd0e into galaxyproject:dev Aug 30, 2017

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
api test Build finished. 284 tests run, 0 skipped, 0 failed.
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 44 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@martenson

This comment has been minimized.

Copy link
Member

commented Aug 30, 2017

Thanks @jmchilton for taking time to polish the test.

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.