Summary:
- remove "make quicktest" and "make memtest"
- when "-j0" is passed to regrtest, use the cpu count detected by multiprocessing
- remove the duplicate test in "make test"
- add "-j0" to the test options in make test

The patch is against default but perhaps we should apply to 3.2 as well.

I propose instead to change 'make quicktest' to use -j(N>1) and blacklist the following tests:

test_mmap
test_shelve
test_posix
test_largefile
test_concurrent_futures

Then (for me) it runs in 3m20s wall clock time which is totally reasonable and I think also still useful.  Note that -j is incompatible with -l so some refactoring of TESTOPTS will probably be required.

> I propose instead to change 'make quicktest' to use -j(N>1) and blacklist the following tests:
> 
> test_mmap
> test_shelve
> test_posix
> test_largefile
> test_concurrent_futures

Why would you blacklist these tests? They are useful.
I agree with Skip's latest message on python-dev: if you blacklist
things you are removing some coverage.
Do note that the most resource-consuming tests are already enabled only
when -usomething is passed.

> Then (for me) it runs in 3m20s wall clock time which is totally
> reasonable and I think also still useful.  Note that -j is
> incompatible with -l so some refactoring of TESTOPTS will probably be
> required.

In the patch I've removed -l, which I've never seen do anything useful.

On Mar 23, 2011, at 03:14 PM, Antoine Pitrou wrote:

>> test_mmap
>> test_shelve
>> test_posix
>> test_largefile
>> test_concurrent_futures
>
>Why would you blacklist these tests? They are useful.

Please keep in mind the use case.  Are these really necessary in a push-race,
post-local-merge, does Python crash-and-burn case?

>I agree with Skip's latest message on python-dev: if you blacklist
>things you are removing some coverage.

Of course, but everyone who develops Python should understand when to run the
appropriate test target <wink>.

>In the patch I've removed -l, which I've never seen do anything useful.

+1.  I think -j is more useful than -l.

-Barry

>- when "-j0" is passed to regrtest, use the cpu count detected by multiprocessing
>- remove the duplicate test in "make test"
>- add "-j0" to the test options in make test

+1. The duplicate test seems quite wasteful (outside of testall). Is there any
reason not to add "-j0" for testall as well?

I think there is merit in keeping the quicktest target, though. Perhaps it
could be renamed to make it clear that it is only meant for use after merge
races? How does "racetest" sound?

> Is there any reason not to add "-j0" for testall as well?

Have you looked at the patch? :)

> Are these really necessary in a push-race,
> post-local-merge, does Python crash-and-burn case?

Yes, they are.
If they are not significant, they should be removed. If they are significant, they should be run.

> Perhaps it
> could be renamed to make it clear that it is only meant for use after > merge
> races? How does "racetest" sound?

Sorry, that's completely bogus. If a "merge race" may introduce a regression, then there's no reason the regression will occur in the non-blacklisted tests. Have you heard of Murphy's law?

On Mar 23, 2011, at 04:06 PM, Antoine Pitrou wrote:

>Sorry, that's completely bogus. If a "merge race" may introduce a regression,
>then there's no reason the regression will occur in the non-blacklisted
>tests. Have you heard of Murphy's law?

That's not the point.  If it was, you'd always have to run make testall
whenever you were resolving a merge race.  Otherwise you could leave out
something important, right?

When you're in the middle of a merge race, you've *already* thoroughly tested
your change, along with the full test suite, with a relatively up-to-date
python tree + your changes <wink>.

You've now merged any changes that have come in since you did your thorough
tests, and you're trying to beat the other guy to the push.  You want
something that can run *fast* and just proves that the merge didn't hose
Python in some brown paper bag way.  It is not intended to be a thorough test
since you've already done that.  Anything more than a smoke test will be
discovered by the buildbots.

> You've now merged any changes that have come in since you did your thorough
> tests, and you're trying to beat the other guy to the push.  You want
> something that can run *fast* and just proves that the merge didn't hose
> Python in some brown paper bag way.

What does "brown paper bag way" mean? It seems to be some kind of urban
legend at this point. A merge won't magically break all C files and
prevent Python from compiling. Especially if no C files were touched in
the first place!

If you are confident that you didn't introduce any issue then just
commit your merge and push (or run the tests which are relevant to your
initial commit).

Oh, and again, if some tests are slow on your system, then *please* open
issues about them (and/or investigate *why* they are slow). That's much
better than ignoring/blacklisting them.

On Mar 23, 2011, at 04:22 PM, Antoine Pitrou wrote:

>What does "brown paper bag way" mean? It seems to be some kind of urban
>legend at this point. A merge won't magically break all C files and
>prevent Python from compiling. Especially if no C files were touched in
>the first place!

This whole thread came up originally because some folks wanted a smoke test
while resolving the merge race window.

>If you are confident that you didn't introduce any issue then just
>commit your merge and push (or run the tests which are relevant to your
>initial commit).

A smoke test addresses the confidence issue, while not introducing a longer
race window to run the full test suite.

>Oh, and again, if some tests are slow on your system, then *please* open
>issues about them (and/or investigate *why* they are slow). That's much
>better than ignoring/blacklisting them.

Sure.

The patch seems to work.

I agree that quicktest and memtest should be removed as well as the duplicate test.

The only thing I would change is to create the number of jobs to be double the cpu count - I think this works quicker.

I don't think the length of testing is so bad on my 3 year old core 2 duo it takes 2:40 to run: EXTRATESTOPTS=-j4 time make test

I committed the "-j0" part of the patch in d8dd7ab6039d.

Brett made the point on #python-dev that a Makefile change doesn't help Windows users. Instead, we may have a Python script somewhere that both "make test" and "make quicktest" call.

I have attached a Python script which does what Antoine's patch does except which is expected to live in Tools/scripts. The perk of doing this in a Python script is that Windows users will be able to simply execute the script while the Makefile can be made to execute the script itself for those that prefer ``make test`` over ``./python Tools/scripts/run_tests.py``.

It tries to have reasonable defaults so that people who do not know what they are doing will have a rigorous test run w/o having it take too long. And the defaults can be overridden easily when people want to do that.

Looking at the actual times with -j0, I don't think there is any need to
keep quicktest - with the removal of the duplicate test, I can do a full
run in 3m16s (on a debug build; non-debug takes 1m54s), which seems plenty
fast enough.

One thing I noticed about the -j option is that it prevents test_curses from
running, since the child process actually running the test isn't connected
to a terminal. I don't know if this is an issue (the curses module doesn't
appear to be under active development), but I thought it was worth pointing
out.

New changeset b76684d62a8d by Nadeem Vawda in branch 'default':
Issue #11651: Improve Makefile test targets.
http://hg.python.org/cpython/rev/b76684d62a8d

> I have attached a Python script which does what Antoine's patch does except
> which is expected to live in Tools/scripts. The perk of doing this in a
> Python script is that Windows users will be able to simply execute the script
> while the Makefile can be made to execute the script itself for those that
> prefer ``make test`` over ``./python Tools/scripts/run_tests.py``.

I've attached a patch that reworks the Makefile test targets to use this script
(with some minor modifications).

Some notes:
- By doing things this way, we lose the ability to specify custom arguments to
  the interpreter with $(TESTPYTHONOPTS). Might this be a problem?
- The "test" and "quicktest" targets now use "-u all,-largefile,-audio,-gui",
  which permits more tests to be run. On my current system, this adds about 20s
  to the running time for "make test" (~3m45s instead of ~3m25s).
- regrtest.py now accepts "-u none", explicitly specifying the default setting
  (to override the setting used by run_tests.py). This isn't strictly necessary,
  but it seemed good to have, for the sake of completeness.
- I've changed the meaning of "-j 1" -- instead of using a single subprocess, it
  runs the tests directly in the current process. This allows us to use the
  run_tests.py script for "make buildbottest" and still have the exact same
  semantics (using even one subprocess can cause problems for e.g. test_curses).

Any thoughts?

> Some notes:
> - By doing things this way, we lose the ability to specify custom arguments to
>   the interpreter with $(TESTPYTHONOPTS). Might this be a problem?

Yes, some buildbots use it. Can't you add support for it in the test
runner?

> - The "test" and "quicktest" targets now use "-u all,-largefile,-audio,-gui",
>   which permits more tests to be run. On my current system, this adds about 20s
>   to the running time for "make test" (~3m45s instead of ~3m25s).

Good idea.

> - I've changed the meaning of "-j 1" -- instead of using a single subprocess, it
>   runs the tests directly in the current process. This allows us to use the
>   run_tests.py script for "make buildbottest" and still have the exact same
>   semantics (using even one subprocess can cause problems for e.g. test_curses).

Ah, nice.

As an aside, the "quicktest" would probably deserve an update.

>> Some notes:
>> - By doing things this way, we lose the ability to specify custom arguments to
>>   the interpreter with $(TESTPYTHONOPTS). Might this be a problem?
>
> Yes, some buildbots use it. Can't you add support for it in the test
> runner?

Working on it now; I'll upload a revised patch shortly.

> As an aside, the "quicktest" would probably deserve an update.

How so? Should it perhaps use "-u none"?

> > As an aside, the "quicktest" would probably deserve an update.
> 
> How so? Should it perhaps use "-u none"?

No, I meant the list of tests that it disables.

What changes do you suggest?

> What changes do you suggest?

Not sure, I never use it. But test_concurrent_futures is not in the list
for example.

Updated patch attached.

>> Some notes:
>> - By doing things this way, we lose the ability to specify custom arguments to
>>   the interpreter with $(TESTPYTHONOPTS). Might this be a problem?
>
> Yes, some buildbots use it. Can't you add support for it in the test runner?

Done.

> But test_concurrent_futures is not in the list for example.

Done.


Out of interest, I measured the running times of the different test targets:
  quicktest      2m25s
  test           3m55s
  testall        8m30s
  buildbottest  14m50s

(these are on a 3-year-old Core 2 Duo laptop)

The latest patch looks ok to me.

New changeset c68a80779434 by Nadeem Vawda in branch 'default':
Issue #11651: Move options for running tests into a Python script.
http://hg.python.org/cpython/rev/c68a80779434

New changeset 6c4fa9559b7e by Nadeem Vawda in branch 'default':
Update README section on testing following issue #11651.
http://hg.python.org/cpython/rev/6c4fa9559b7e