classification
Title: Allow all resources if not running under regrtest.py
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: ezio.melotti, python-dev, r.david.murray, serhiy.storchaka, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2013-07-18 15:38 by zach.ware, last changed 2014-06-02 23:02 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
regrtest_run.diff zach.ware, 2013-07-18 15:38 review
issue18492.v2.diff zach.ware, 2014-05-07 17:29 review
Messages (11)
msg193310 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-07-18 15:38
Here's a patch to implement the idea I posted in issue18258, msg193242.  The patch also removes usage of "support.use_resources = ['<resource>']" from the test package since it is no longer needed.

(No test_main -> unittest.main conversions are done in this patch; the modules changed that still us. test_main are either covered by another issue or are more complex than a simple remove-and-replace conversion, and so I didn't want to lump them into this patch)
msg193314 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-07-18 19:03
I don't think we should accept anything if not running under regrtest, We can unintentionally run a testfile in which some tests consume a lot of resources. Also we need in skipping this tests for the test testing. I think it will be better just add a support of the -u option to discovery command line interface.
msg193360 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-19 14:44
Since we only use unittest for running subsets of our test suite, I think the better behavior is to run everything that -u all normally runs. That is, when one uses unittest to run a bit of the test suite, one generally wants to run the tests one specifies, not have them skipped.

Obviously this is open to discussion, however ;)
msg193386 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-20 00:17
Currently, all requires() tests pass when the file they occur in is run  as '__main__'. This is especially needed when the file ends with the now standard boilerplate.
  if __name__ == '__main__':
      ...
      unittest.main(...)
as there is currently no way to set resources within the unittest.main call.

The problem is that this permissiveness does not apply to subsidiary files discovered from and run by a main file, even though it should. The current workaround is to explicitly set use_resources for the benefit of subsidiary files, as in test_idle.py.

As I see it, the main point of this patch, somewhat obscured by the title, is to extend the current resource permissiveness from tests *in* main files run as main to tests in other files discovered and run from a main file. It also extends the permisiveness to any test not run by regrtest (ie, by unittest).

The key change is
-    if sys._getframe(1).f_globals.get("__name__") == "__main__":
+    if not regrtest_run:

'regrtest_run == True' is currently spelled 'use_resources is not None', so the latter could be used instead to replace the frame-check without otherwise adding new code. 

Extending the permissiveness from main files to subsidiary files strikes me as a no-brainer. Splitting a (potentially) large file into a master file and a package of subsidiary files should not affect which tests are run.

More interesting is extending the permisiveness to tests run under unittest with "python -m unittest target". Target can be a master file, a test file, or a test case or test methods. Subfile targets can only be run with unittest, not regrtest, and there is no way to enable resources for such targets.
  python -m unittest idlelib.idle_test.test_xy.TextText # runs
  python -m unittest idlelib.idle_test.test_xy.GuiText # skips
So the patch enables something that is currently not possible.

Serhiy is concerned about the possible booby-trap for users that run slow resource intensive tests. Some thoughts:

* Running tests as main is mainly done interactively, and usually by developers at that. Humans can stop a test and ignore errors better than buildbots.

* There are multiple ways to run a file from the command line. The test chapter of the manual can document that 
  python -m test.test_xyz
is more or less equivalent to
  python -m test -uall test_zyz
with -v possibly tossed in. Anyone who does not want that can still run under regrtest by using the currently documented
  python -m test test_xyz

* Anyone running a test file loaded in an Idle window very likely wants to run all tests possible.

* Documenting that running under unittest enables all resources is trickier as long as resources are cpython and regrtest specific. I think I would mention this in the test chapter, where resources are discussed, rather than the unittest chapter.

*If -u is added to unittest (and 'use=' to .main), a default of all would be the right thing for subfile targets, even if not for file and multi-file targets.
---
msg193387 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-20 00:25
#18441 is partly related in that it also suggests (eventually) moving check code (for guis) from multiple test files to support and regrtest. I do not believe there would be any conflict.
msg193494 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-07-22 04:11
Terry J. Reedy added the comment:
> The problem is that this permissiveness does not apply to subsidiary files discovered from and run by a main file, even though it should. The current workaround is to explicitly set use_resources for the benefit of subsidiary files, as in test_idle.py.

This is exactly what inspired this issue, though with
test_codecmaps_*.py rather than test_idle.

> As I see it, the main point of this patch, somewhat obscured by the title, is to extend the current resource permissiveness from tests *in* main files run as main to tests in other files discovered and run from a main file. It also extends the permisiveness to any test not run by regrtest (ie, by unittest).

I did choose the title rather poorly, but yes, this is the real point.

> The key change is
> -    if sys._getframe(1).f_globals.get("__name__") == "__main__":
> +    if not regrtest_run:
>
> 'regrtest_run == True' is currently spelled 'use_resources is not None', so the latter could be used instead to replace the frame-check without otherwise adding new code.

I think using the 'regrtest_run' flag is more explicit about why we're
just letting anything go, and could potentially be useful in other
situations as well (though I'm coming up blank on what any of those
might be right now).

<snip>

> Serhiy is concerned about the possible booby-trap for users that run slow resource intensive tests. Some thoughts:
<snip thoughts>

I agree with your thoughts here, which if I'm not mistaken basically
boil down to "Yes, it's a booby-trap, but it's not a lethal one and we
can document around most of it."

> *If -u is added to unittest (and 'use=' to .main), a default of all would be the right thing for subfile targets, even if not for file and multi-file targets.

I'm not sure I understand this line.  But I do have an initial patch
adding -u to unittest which I will be posting shortly which, if
accepted, should render this issue moot.
msg218064 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-05-07 17:29
Here's a new and better patch.  This patch keeps the regrtest_run global in support and moves the regrtest-or-not check into is_resource_enabled to make is_resource_enabled, requires, and requires_resource consistent with each other and in a way that still allows explicitly setting support.use_resources.  The real change is confined to Lib/test/regrtest.py and Lib/test/support/__init__.py, the rest of the patch is cleanup allowed by the change (except for test_decimal, which has a minor change required to allow one of the command line options to that script to work).
msg218509 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-05-14 08:33
You have convinced me. In general the approach and the patch LGTM.

But I agree with Terry that flag support.regrtest_run is redundant, it duplicates a bit of information which provides support.use_resources. Instead we can rewrite support.is_resource_enabled() as

    return use_resources is None or resource in use_resources

Introducing regrtest_run can cause inconsistency between it and use_resources.
msg219639 - (view) Author: Roundup Robot (python-dev) Date: 2014-06-02 21:05
New changeset eabff2a97b5b by Zachary Ware in branch '2.7':
Issue #18492: Allow all resources when tests are not run by regrtest.py.
http://hg.python.org/cpython/rev/eabff2a97b5b

New changeset 8519576b0dc5 by Zachary Ware in branch '3.4':
Issue #18492: Allow all resources when tests are not run by regrtest.py.
http://hg.python.org/cpython/rev/8519576b0dc5

New changeset 118e427808ce by Zachary Ware in branch 'default':
Issue #18492: Merge with 3.4
http://hg.python.org/cpython/rev/118e427808ce
msg219640 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-06-02 21:07
You convinced me too, Serhiy :).  Committed, without the regrtest_run flag.  Thanks for review.
msg219649 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-02 23:02
Just in time, as I will be reviewing and committing new Idle unittest module starting this week (new GSOC student).
History
Date User Action Args
2014-06-02 23:02:59terry.reedysetmessages: + msg219649
2014-06-02 21:07:35zach.waresetstatus: open -> closed
resolution: fixed
messages: + msg219640

stage: patch review -> resolved
2014-06-02 21:05:45python-devsetnosy: + python-dev
messages: + msg219639
2014-06-01 15:17:53serhiy.storchakasetassignee: zach.ware
2014-05-14 08:33:55serhiy.storchakasetmessages: + msg218509
2014-05-07 17:29:40zach.waresetfiles: + issue18492.v2.diff

title: Add test.support.regrtest_run flag, simplify support.requires -> Allow all resources if not running under regrtest.py
messages: + msg218064
versions: + Python 2.7, Python 3.5, - Python 3.3
2013-07-22 04:11:01zach.waresetmessages: + msg193494
2013-07-20 00:25:26terry.reedysetmessages: + msg193387
2013-07-20 00:17:47terry.reedysetnosy: + terry.reedy

messages: + msg193386
stage: patch review
2013-07-19 14:44:01r.david.murraysetmessages: + msg193360
2013-07-18 19:03:52serhiy.storchakasetmessages: + msg193314
2013-07-18 15:38:20zach.warecreate