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

gh-108303: Move zip and tar archives to Lib/test/archivetestdata #111549

Merged
merged 4 commits into from
Nov 4, 2023

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 31, 2023

All archives now live in the same place.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

It's surprising that "ziptestdata" is used by test_tarfile and contains TAR archives. Maybe rename the directory to "archivetestdata"?

@sobolevn sobolevn requested a review from 1st1 as a code owner October 31, 2023 14:40
@vstinner vstinner changed the title gh-108303: Move zip and tar archives to Lib/test/ziptestdata gh-108303: Move zip and tar archives to Lib/test/archivetestdata Oct 31, 2023
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Lib/test/archivetestdata/README.md says "Test data for test_zipfile". Maybe mention somewhere that now it's also used by test_tarfile, and also test_contextlib?

Also, the README file says "Purpose: These are used to test executable files with an appended zipfile". Is it still the case?

@vstinner
Copy link
Member

If some test files are specific to "test executable files with an appended zipfile", maybe they should live in a separated directory? I'm not sure if it's worth it. Maybe just list files related to these specific tests?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Can you please try to fix the merge conflict?

@sobolevn
Copy link
Member Author

sobolevn commented Nov 4, 2023

Done!

@vstinner vstinner merged commit 890ef1b into python:main Nov 4, 2023
25 checks passed
@vstinner
Copy link
Member

vstinner commented Nov 4, 2023

Merged, thanks. I don't know how feasible it is to backport the change to 3.11 and 3.12? @sobolevn: try maybe to add the backport labels?

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants