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

python setup.py test -n option is broken #2566

Merged
merged 2 commits into from Jun 1, 2014
Merged

Conversation

cdeil
Copy link
Member

@cdeil cdeil commented May 27, 2014

python setup.py test --help claims that one can use the -n option instead of --parallel:

$ python setup.py test --help
...
  --parallel (-n)         Run the tests in parallel on the specified number of
                          CPUs.  If negative, all the cores on the machine
                          will be used.  Requires the pytest-xdist plugin.
...

But actually this doesn't work with current astropy master:

$ python setup.py test -n
...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "astropy/__init__.py", line 196, in test
    skip_docs=skip_docs)
  File "astropy/tests/helper.py", line 231, in run_tests
    "parallel must be an int, got {0}".format(parallel))
ValueError: parallel must be an int, got 
$ python setup.py test -n 4
invalid command name '4'
$ python setup.py test -n4
error: option -4 not recognized

Am I using it incorrectly or is this broken.
(I guess the -n shortcut could simply be removed?)

@mdboom
Copy link
Contributor

mdboom commented May 27, 2014

Ah -- I see why this isn't working: it conflicts with the distutils built-in option of "dry-run", which is also "-n". We could change this to -j, to be consistent with make.

@embray
Copy link
Member

embray commented May 27, 2014

Never caught that due to usually just using --parallel. +1 to -j.

cdeil added a commit to cdeil/astropy that referenced this pull request May 27, 2014
@cdeil
Copy link
Member Author

cdeil commented May 27, 2014

I've attached a commit that changes to -j in astropy.
The same change should be made in astropy-helpers.

@embray Can you cherry-pick that commit into astropy-helpers directly or should I make an extra PR there?

@embray
Copy link
Member

embray commented May 27, 2014

Either way.

@cdeil
Copy link
Member Author

cdeil commented May 27, 2014

Then I'll admit I'm lazy and not make a separate PR for astropy-helpers.

This change is not tested on travis-ci anyways, so is this ready to merge?

@astrofrog
Copy link
Member

Actually should we even change it in astropy at all? Or should all changes be made only in astropy-helpers?

@embray
Copy link
Member

embray commented May 28, 2014

Both, for now.

@cdeil
Copy link
Member Author

cdeil commented May 28, 2014

I think the travis-ci fail is unrelated to this PR?
https://travis-ci.org/astropy/astropy/jobs/26160813
Can someone please restart that build?

@cdeil
Copy link
Member Author

cdeil commented May 28, 2014

@embray Can you please label this "bug" and add the 0.4.0 or 0.4.1 milestone?

@embray embray added this to the v0.4.0 milestone May 29, 2014
@embray
Copy link
Member

embray commented May 29, 2014

This is pretty minor, but if you could just add a changelog entry (perhaps under "misc"?) then we can merge this.

cdeil added a commit to cdeil/astropy that referenced this pull request May 29, 2014
@cdeil
Copy link
Member Author

cdeil commented May 29, 2014

I added a changelog entry.

@astrofrog
Copy link
Member

@cdeil - it looks like this needs rebasing

@cdeil
Copy link
Member Author

cdeil commented May 31, 2014

rebased

astrofrog added a commit that referenced this pull request Jun 1, 2014
python setup.py test -n option is broken
@astrofrog astrofrog merged commit fca0415 into astropy:master Jun 1, 2014
@cdeil
Copy link
Member Author

cdeil commented Jun 1, 2014

@astrofrog This fix needs to be merged into astropy-helpers, too.

@astrofrog
Copy link
Member

@cdeil - could you open a pull request there? (also, I realized too late that this was in astropy-helpers - and should only really be fixed there)

@cdeil
Copy link
Member Author

cdeil commented Jun 1, 2014

Here's the PR for astropy-helpers: astropy/astropy-helpers#29

@embray embray modified the milestones: v0.4.0, v0.4.1 Jun 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants