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
Allow all resources if not running under regrtest.py #62692
Comments
Here's a patch to implement the idea I posted in bpo-18258, 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) |
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. |
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 ;) |
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. 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
'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. Serhiy is concerned about the possible booby-trap for users that run slow resource intensive tests. Some thoughts:
*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. |
bpo-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. |
Terry J. Reedy added the comment:
This is exactly what inspired this issue, though with
I did choose the title rather poorly, but yes, this is the real point.
I think using the 'regrtest_run' flag is more explicit about why we're <snip>
I agree with your thoughts here, which if I'm not mistaken basically
I'm not sure I understand this line. But I do have an initial patch |
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). |
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. |
New changeset eabff2a97b5b by Zachary Ware in branch '2.7': New changeset 8519576b0dc5 by Zachary Ware in branch '3.4': New changeset 118e427808ce by Zachary Ware in branch 'default': |
You convinced me too, Serhiy :). Committed, without the regrtest_run flag. Thanks for review. |
Just in time, as I will be reviewing and committing new Idle unittest module starting this week (new GSOC student). |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: