Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commented Oct 31, 2023

All archives now live in the same place.

Copy link
Member

@vstinnervstinner 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"?

@sobolevnsobolevn requested a review from 1st1 as a code ownerOctober 31, 2023 14:40
@vstinnervstinner changed the title gh-108303: Move zip and tar archives to Lib/test/ziptestdatagh-108303: Move zip and tar archives to Lib/test/archivetestdataOct 31, 2023
Copy link
Member

@vstinnervstinner 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

@vstinnervstinner 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
MemberAuthor

Done!

@vstinnervstinner merged commit 890ef1b into python:mainNov 4, 2023
@vstinner
Copy link
Member

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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sobolevn@vstinner