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

Fix flaky bug in test_ert_qstat_proxy #3731

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

berland
Copy link
Contributor

@berland berland commented Aug 10, 2022

The subprocesses were not allowed to finished before the backend script was removed
from path (provided by the testpath context manager)

Issue
Resolves unreported flakyness in test script for qstat proxy.

Approach
Extend lifetime of context manager

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@berland berland added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Aug 10, 2022
@berland berland changed the title Fix flakybug in test_ert_qstat_proxy Fix flaky bug in test_ert_qstat_proxy Aug 10, 2022
The subprocesses were not allowed to finished before the backend script was removed
from path (provided by the testpath context manager)

Also invert the MacOS test in the proxy script, so that it
will fall back to backend on anything but Linux.
@berland berland marked this pull request as ready for review August 10, 2022 09:52
if [[ $OSTYPE == "darwin"* ]]; then
# No sufficient file locking mechanism available on Mac OS X. Fallback:
if [ `uname` != "Linux" ]; then
# Fallback if we are not on Linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't sure that it works, but it seems it does :)

Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

Nice! 🚀

@berland
Copy link
Contributor Author

berland commented Aug 10, 2022

Jenkins test required please

@berland berland enabled auto-merge (rebase) August 10, 2022 13:39
@berland berland merged commit f9042c7 into equinor:main Aug 10, 2022
@berland berland deleted the fix_proxy_flakyness branch October 18, 2022 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants