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

make test_find_eb_script more robust in case $EB_SCRIPT_PATH is already set #3320

Merged
merged 2 commits into from
May 2, 2020

Conversation

boegel
Copy link
Member

@boegel boegel commented May 1, 2020

No description provided.

@boegel boegel added the bug fix label May 1, 2020
@boegel boegel added this to the next release (4.2.1?) milestone May 1, 2020
@boegel boegel requested a review from akesandgren May 1, 2020 16:20
@@ -2027,19 +2027,35 @@ def makedirs_in_test(*paths):

def test_find_eb_script(self):
"""Test find_eb_script function."""

# make sure $EB_SCRIPT_PATH is not set already (used as fallback mechanism in find_eb_script)
if 'EB_SCRIPT_PATH' in os.environ:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to reset this to the original value at the end of this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

The environment is already automatically reset to the original at the end of each test (see tearDown in test/framework/utilities.py).

@easybuilders easybuilders deleted a comment from boegelbot May 1, 2020
Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Contributor

Going in, thanks @boegel!

@akesandgren akesandgren merged commit ed5ba4a into easybuilders:develop May 2, 2020
@boegel boegel deleted the test_find_eb_script branch May 2, 2020 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants