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

only restore $PYTHONPATH if it was defined #743

Merged
merged 2 commits into from Nov 21, 2015

Conversation

boegel
Copy link
Member

@boegel boegel commented Nov 16, 2015

No description provided.

@boegel boegel added this to the v2.5.0 milestone Nov 16, 2015
@hpcugentbot
Copy link
Contributor

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1364/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Contributor

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1365/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@JensTimmerman
Copy link
Contributor

can we have a test for this?

@boegel
Copy link
Member Author

boegel commented Nov 16, 2015

We don't have runtime tests for easyblocks in the unit tests. This type of thing can really only be tested by doing a full build, which is done during regression testing.

However, there's something going on here that I don't fully understand. Although the fix is needed, someone (see EB ML) ran into this while $PYTHONPATH was defined, so the None case should never have happened...

@JensTimmerman
Copy link
Contributor

wouldn't it not make sense to have some unittests for the generic EasyBlocks?

@boegel
Copy link
Member Author

boegel commented Nov 20, 2015

@JensTimmerman: you mean like, creating small toy programs that are built using the generic easyblocks? it can get tricky expensive, for example if CMake is involved, or for OCamlPackage, etc...

We should do proper testing with the actual applications instead when updating easyblocks (which I typically do when an existing easyblock is touched).

@boegel
Copy link
Member Author

boegel commented Nov 20, 2015

@wpoely86: please review?

@wpoely86
Copy link
Member

looks good

@boegel
Copy link
Member Author

boegel commented Nov 21, 2015

Thanks @wpoely86!

boegel added a commit that referenced this pull request Nov 21, 2015
only restore $PYTHONPATH if it was defined
@boegel boegel merged commit d75dc16 into easybuilders:develop Nov 21, 2015
@boegel boegel deleted the eb_fix_PYTHONPATH branch November 21, 2015 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants