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

Fix `PytestRun` passthru arg handling. #5594

Merged
merged 4 commits into from Mar 15, 2018

Conversation

Projects
None yet
3 participants
@jsirois
Copy link
Member

jsirois commented Mar 13, 2018

Previously passthru arguments were shlex-split even though they were
already presented in list form via --options and the passthru args
facility. Add a unit test to confirm passthru args are not over-split
and deprecate --options in favor of the standard passthru args
facility.

@jsirois jsirois requested review from benjyw , kwlzn and dotordogh Mar 13, 2018

@jsirois

This comment has been minimized.

Copy link
Member

jsirois commented Mar 13, 2018

N.B.: A yak shaved while working test failures in another branch, where I discovered you had to say:

./pants test tests/python/pants_test/backend/python/tasks -- -k'"test_cache_greens_slow or test_fail_fast_skips_second_red_test_with_isolated_chroots or test_out_of_band_deselect_no_fast_success"'

Crazy double-quoting no longer required, just enough quoting to work with the shell, which is what you'd expect.

register('--options', type=list, fingerprint=True, help='Pass these options to pytest.')
register('--options', type=list, fingerprint=True,
removal_version='1.7.0.dev0',
removal_hint='You can supply py.test options using the generic pass through the args '

This comment has been minimized.

@benjyw

benjyw Mar 13, 2018

Contributor

s/pass through the args/pass through args/

This comment has been minimized.

@jsirois

jsirois Mar 13, 2018

Member

Fixed

@benjyw

benjyw approved these changes Mar 13, 2018

Copy link
Contributor

benjyw left a comment

Nice test!

@kwlzn

kwlzn approved these changes Mar 13, 2018

@jsirois jsirois force-pushed the jsirois:PytestRun/FixPassthru branch 3 times, most recently from 60a56ea to 6d31cb4 Mar 14, 2018

jsirois added some commits Mar 13, 2018

Fix `PytestRun` passthru arg handling.
Previously passthru arguments were shlex-split even though they were
already presented in list form via `--options` and the passthru args
facility. Add a unit test to confirm passthru args are not over-split
and deprecate `--options` in favor of the standard passthru args
facility.

@jsirois jsirois force-pushed the jsirois:PytestRun/FixPassthru branch from 6d31cb4 to 85814c7 Mar 15, 2018

@jsirois jsirois merged commit e5d22e3 into pantsbuild:master Mar 15, 2018

1 check passed

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

@jsirois jsirois deleted the jsirois:PytestRun/FixPassthru branch Mar 15, 2018

baroquebobcat added a commit that referenced this pull request May 8, 2018

[pytest runner] re-add --options flag as a shlexed list of strings (#…
…5790)

### Problem

`--options` was removed from pytest when cleaning up some issues with passthrough args (#5594). This made it more difficult to pass arbitrary args for pytest runs for automated use cases or to have options set in `pants.ini`. It makes it more difficult because now those options must be collated and appended to the end of the command in all cases. If the command construction code splits up how different flags are populated, it can be difficult to ensure that a particular flag's contents always ends up at the end.

### Solution

Bring back the options flag, but only shlex it and not the passthrough args.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment