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

--fast flag for test suite! #4167

Merged
merged 5 commits into from Sep 4, 2013
Merged

Conversation

ivanov
Copy link
Member

@ivanov ivanov commented Sep 3, 2013

runs the test suite in parallel, closes #1880

@minrk
Copy link
Member

minrk commented Sep 3, 2013

don't forget to rebase on PR #4165.

@ivanov
Copy link
Member Author

ivanov commented Sep 3, 2013

actually just updated to be standalone. This PR doesn't interfere with #4165

nose_pkg_names.append('kernel')
nose_pkg_names.append('kernel.inprocess')
nose_pkg_names.insert(0, 'kernel')
nose_pkg_names.insert(1, 'kernel.inprocess')
if inc_slow:
nose_pkg_names.append('parallel')
Copy link
Member

Choose a reason for hiding this comment

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

parallel is the slowest of all, shouldn't it be one of the first as well?

@ivanov
Copy link
Member Author

ivanov commented Sep 3, 2013

the output now looks something like this:

**********************************************************************

IPython test group: IPython.kernel..................................OK

IPython test group: IPython.kernel.inprocess........................OK

IPython test group: IPython.config..................................OK

IPython test group: IPython.core....................................OK

IPython test group: IPython.extensions..............................OK

IPython test group: IPython.lib.....................................OK

IPython test group: IPython.terminal................................OK

IPython test group: IPython.testing.................................OK

IPython test group: IPython.utils...................................OK

IPython test group: IPython.nbformat................................OK

IPython test group: IPython.qt......................................OK

IPython test group: IPython.html....................................OK

IPython test group: IPython.nbconvert...............................OK

IPython test group: autoreload......................................OK

**********************************************************************

and iptest --fast finishes in 22 seconds, whereas without the --fast flag, it takes 75 seconds here.

@ivanov
Copy link
Member Author

ivanov commented Sep 3, 2013

and with iptest --all --fast I get 70 seconds, whereas without it, it's 217.261.

@takluyver
Copy link
Member

Looks good to me.

@bfroehle
Copy link
Contributor

bfroehle commented Sep 4, 2013

This is a wonderful addition, and with minimal work would probably suffice to close #1880 which I opened a year ago.

The only thing I'd consider changing is the name of the flag, from --fast which is pretty ambiguous to -j [jobs] / --jobs=[jobs] as is standard in most common unix tools:

  • iptest (use 1 process)
  • iptest -j (use all available processes, or, perhaps, start all subtests simultaneously)
  • iptest -j 4 (use 4 processes)

Does anybody else have thoughts on the name?

@takluyver
Copy link
Member

@ivanov @minrk and I discussed the flag name in person - adding a parameter would require proper argument parsing, whereas iptest is currently just doing if '--fast' in sys.argv, so we said we'd leave that for when I refactor iptest, which I plan to start working on once this lands.

@ivanov
Copy link
Member Author

ivanov commented Sep 4, 2013

Yeah, I'm all for a -j flag going forward, it was just going to make this code a lot uglier on the parser side of things, as @takluyver pointed out. In the meantime, the --fast flag works eponymously, and can later be an alias for just -j when the number of threads is specified. I added that it closes #1880 to the PR description, so GH can do it's magic in auto-closing that when this lands

print('*'*70)
print('IPython test group:',name)
res = runner.run()
all_res = p.map(do_run, runners)
Copy link
Member

Choose a reason for hiding this comment

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

do you want to use imap_unordered, so that results will arrive as they finish? They are still executed in the same order.

Copy link
Member

Choose a reason for hiding this comment

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

The zip() just below currently relies on getting the results in the same order that they were submitted, though I'm sure there are ways around that if we want to use imap_unordered.

Copy link
Member

Choose a reason for hiding this comment

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

okay, then no big deal. But if @ivanov wants it, if do_run just returned (x[0], x[1], ret), everything would be in sync and the zip would be unnecessary.

takluyver added a commit that referenced this pull request Sep 4, 2013
@takluyver takluyver merged commit f435e6b into ipython:master Sep 4, 2013
@ivanov ivanov deleted the parallel-tests branch September 5, 2013 00:44
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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.

Add parallelism to test_pr
4 participants