Skip to content

bpo-38223: Reorganize test_shutil. #16281

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

Merged
merged 1 commit into from
Sep 26, 2019

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 19, 2019

  • Group tests for specific functions and groups of related functions
    into separate classes.
  • Clean up creating and cleaning up temporary directories.
  • Simplify and make more robust monkey patching of shutil.open.

https://bugs.python.org/issue38223

* Group tests for specific functions and groups of related functions
into separate classes.
* Clean up creating and cleaning up temporary directories.
* Simplify and make more robust monkey patching of shutil.open.
@ned-deily
Copy link
Member

Why would this be backported to maintenance releases?

@serhiy-storchaka
Copy link
Member Author

This will help a much in backporting future tests. It is easier and safer to backport large but straightforward changes which will allow future backports to be made automatically by @miss-islington or manually with simple conflicts than backport all future changes only manually and with large conflicts and large risk of errors. It is common practice for large test changes.

I am not sure about 2.7. If the code is too different (because shutil have many new features in Python 3 and different techniques are used for testing), it may be not worth to backport changes.

@ned-deily
Copy link
Member

ned-deily commented Sep 22, 2019

It's good that you want to improve the code base but why would we be doing future backporting of tests to maintenance branches nearing the end of their bugfix periods? We should be minimizing the number and scope of changes going into 3.7 and especially 2.7 at this point in their cycles. And, in any case, large-scale refactoring of code, be it in library modules or tests, is out-of-scope for maintenance branches. The devguide is clear: "The only changes allowed to occur in a maintenance branch without debate are bug fixes." As releases mature in their bugfix phase, our downstream users expect us to not break anything so that they can painlessly as possible update to the latest maintenance release, and that includes tests which some users run as part of their testing programs. At this point in their cycles, we should be focused on security fixes, regressions within a release cycle, bugs that are still currently causing problems for users, and any changes necessary to support updates to the primary supported platforms.

@giampaolo
Copy link
Contributor

I concur it's probably not worth the backport(s), otherwise +1 on the change.

@serhiy-storchaka
Copy link
Member Author

Okay. But in past we usually backported test reorganizing changes to help future backporting of tests for bugfixes. We also backported documentation changes. We even backported such significant changes as rewriting test.regress and test.support modules (they are packages now). All for simplifying future backports. It did not break compatibility, as it is a code used primarily by core developers.

It is not without debate. There is a debate.

@serhiy-storchaka serhiy-storchaka merged commit 4f2eac0 into python:master Sep 26, 2019
@serhiy-storchaka serhiy-storchaka deleted the test_shutil branch September 26, 2019 10:15
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
* Group tests for specific functions and groups of related functions
into separate classes.
* Clean up creating and cleaning up temporary directories.
* Simplify and make more robust monkey patching of shutil.open.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants