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

Ensure test goal implicitly targets current platform when using python_dist targets #5720

Merged
merged 3 commits into from Apr 18, 2018

Conversation

Projects
None yet
3 participants
@CMLivingston
Copy link
Contributor

CMLivingston commented Apr 17, 2018

Problem

#5618 introduced safety checks to ensure that targets depending on python_dist targets build for the current platform only. However, test targets do not have a platform parameter, and we need to extend this check to ensure that ./pants test runs do not inherit incompatible default platform constraints and only build for 'current' only.

Solution

Give the python_dist target a hard-coded platform constraint so that python_tests implicitly targets the current platform when testing APIs from a python_dist target.

Result

Users can now depend on the python_dist target in python_tests targets when pants.ini specifies multiple platforms.

Chris Livingston added some commits Apr 17, 2018

@CMLivingston CMLivingston changed the title Add support for test goal with python_dists Ensure test goal implicitly targets current platform when using `python_dist`s Apr 18, 2018

@CMLivingston CMLivingston changed the title Ensure test goal implicitly targets current platform when using `python_dist`s Ensure test goal implicitly targets current platform when using python_dist targets Apr 18, 2018

if tgt.platforms:
for platform in tgt.platforms:
if platform in tgts_by_platforms:
tgts_by_platforms[platform].append(tgt)
else:
tgts_by_platforms[platform] = [tgt]

for tgt in tgts:
insert_or_append_tgt_by_platform(tgt)

This comment has been minimized.

@wisechengyi

wisechengyi Apr 18, 2018

Contributor

if you mean to adopt the functional style like filter(predicate, tgts), you can probably use
map(insert_or_append_tgt_by_platform, tgts)?

Chris Livingston
Address @wisechengyi's comment. Use a platform specific string for ne…
…w test to deal with sdist translating for a target platform.

@wisechengyi wisechengyi merged commit 19f7aa8 into pantsbuild:master Apr 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

wisechengyi added a commit that referenced this pull request Apr 18, 2018

Ensure test goal implicitly targets current platform when using pytho…
…n_dist targets (#5720)

### Problem

#5618 introduced safety checks to ensure that targets depending on `python_dist` targets build for the current platform only. However, test targets do not have a platform parameter, and we need to extend this check to ensure that `./pants test` runs do not inherit incompatible default platform constraints and only build for `'current'` only.

### Solution

Give the `python_dist` target a hard-coded platform constraint so that `python_tests` implicitly targets the current platform when testing APIs from a `python_dist` target.

### Result

Users can now depend on the `python_dist` target in `python_tests` targets when pants.ini specifies multiple platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment