Skip to content

fix unit tests by not cleaning up all $EASYBUILD_X env vars, only the ones that matter#963

Merged
boegel merged 3 commits intoeasybuilders:developfrom
boegel:fix_unit_tests
Jul 3, 2014
Merged

fix unit tests by not cleaning up all $EASYBUILD_X env vars, only the ones that matter#963
boegel merged 3 commits intoeasybuilders:developfrom
boegel:fix_unit_tests

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Jul 3, 2014

No description provided.

wpoely86
wpoely86 reviewed Jul 3, 2014
View reviewed changes
Comment thread easybuild/tools/modules.py
Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Jul 3, 2014

Choose a reason for hiding this comment

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

Shouldn't we restore the env as it was before EB started if we do stuff like this?

Copy link
Copy Markdown
Member Author

@boegel boegel Jul 3, 2014

Choose a reason for hiding this comment

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

this change is local to the running eb process, it can't leak into the session in which eb was run...

@wpoely86
Copy link
Copy Markdown
Member

wpoely86 commented Jul 3, 2014

Looks fine

boegel
boegel reviewed Jul 3, 2014
View reviewed changes
Comment thread easybuild/tools/modules.py
Copy link
Copy Markdown
Member Author

@boegel boegel Jul 3, 2014

Choose a reason for hiding this comment

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

@pforai, @pescobar: are you OK with this, bumping the required Lmod version to the just released v5.6.3 (for the upcoming EasyBuild v1.14.0)? There are a couple of critical fixes in that version, on which the current develop version of framework depends (depending on the configuration, but doing different version checks based on how EB is configured is not something I want to get into)

Copy link
Copy Markdown
Contributor

@pforai pforai Jul 3, 2014

Choose a reason for hiding this comment

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

Agreed! That version brings us additional benefits.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jul 3, 2014

heads up: I've set the required version of modulecmd to 3.2.10 in this too, to avoid that people get surprised by nasty issues with older versions (cfr. easybuilders/easybuild#29)

@pescobar
Copy link
Copy Markdown
Member

pescobar commented Jul 3, 2014

@boegel this will be a good excuse to upgrade my Lmod installs :)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jul 3, 2014

Thanks all for reviewing & feedback, merging this in!

@wpoely86, @pescobar: after this is merged in, just merge in develop into your branch and update your PRs to make Jenkins give a green light

boegel added a commit that referenced this pull request Jul 3, 2014
@boegel
Merge pull request #963 from boegel/fix_unit_tests
d81e0c2 
fix unit tests by not cleaning up all $EASYBUILD_X env vars, only the ones that matter
@boegel boegel merged commit d81e0c2 into easybuilders:develop Jul 3, 2014
@boegel boegel mentioned this pull request Jun 24, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@boegel @wpoely86 @pescobar @pforai