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

Mock PATH for problematic interpreter selection test in CI #474

Merged
merged 17 commits into from
May 14, 2018

Conversation

CMLivingston
Copy link
Contributor

@CMLivingston CMLivingston commented May 13, 2018

Fixes #470.

@CMLivingston CMLivingston changed the title Fix .travis.yml path setting for testing purposes Mock PATH for problematic interpreter selection test in CI May 14, 2018
@CMLivingston
Copy link
Contributor Author

@kwlzn This is ready for a look when you can. The only remaining issue is that the refactored interpreter test helper is failing on the pypy shard with a buffer overflow error. Not sure about what could be causing it...do you have any ideas?

.travis.yml Outdated
# The pypy test shard fails with "too many open files" under Trusty so we pin to Precise.
dist: precise
dist: trusty
Copy link
Contributor

Choose a reason for hiding this comment

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

would probably be better to make this change in a separate PR, I think.

res.assert_success()
pex_info = get_pex_info(pex_out_path)
assert ['>3'] == pex_info.interpreter_constraints
py3_interpreter = ensure_python_interpreter('3.6.3')
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than mocking out os.getenv here, how about just temporarily setting the PATH env var with twitter.common.contextutil.environment_as for the scope of this test?

@@ -343,17 +349,19 @@ def test_interpreter_constraints_to_pex_info_py2():
assert set(['>=2.7', '<3']) == set(pex_info.interpreter_constraints)


@pytest.mark.skip('https://github.com/pantsbuild/pex/issues/470')
def test_interpreter_constraints_to_pex_info_py3():
Copy link
Contributor

Choose a reason for hiding this comment

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

test_pex_python in test_integration.py needs the same treatment as this test - both were disabled.

Copy link
Contributor

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

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

lgtm

@kwlzn kwlzn merged commit 069fa2e into pex-tool:master May 14, 2018
This was referenced May 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants