-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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 all XML-related test files to test_xml
#114344
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_xmlrpc and test_docxmlrpc are not XML tests.
Hum, I don't get the PR description. So, what it does:
It changes also findfile() of test.support: if subdir is not a string, use What do you think about removing |
I do not think that it is worth to do. Unlike to |
@serhiy-storchaka sorry, I don't agree. They might have any implementation details, but they are still sharing the same namespace and they use the same fixture files. It is logical for test to have the same namespaces as regular code does. |
af1b057
to
13fdbc7
Compare
@vstinner I tried to remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to close this issue.
There are different aspects:
- Is it possible to run individual tests separately? Like
test_xml.test_test_minidom
. Yes, it's possible. - Does the change increase parallelism of tests, or make it worse? Same, since regrtest splits
test_xml
into sub-tests to run them in parallel. - Are these sub-tests related? How? Well, there are related to "XML". Their implementations are unrelated.
xml.minidom
implementation has nothing to do withxml.etree
.
they are still sharing the same namespace and they use the same fixture files. It is logical for test to have the same namespaces as regular code does.
We have many "os-like" and "socket-like" tests in separated tests, and that's why we have helpers such as support.os_helper
and support.socket_helper
. I don't buy the argument "putting everything at the same place is better".
While it's very tempting to put "everything related to XML" in a test package, for this change, I prefer to stick to the status quo: "flat namespace is better than nested" (PEP 20) :-)
Not changing tests keeps the Git history linear (no custom command line option needed) and avoids developers to change their habits. I don't think that the benefits are way higher than drawbacks (moving code).
Test files are already in the xmltestdata
directory.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Ok, sounds convincing 👍 |
There should be a lot of changed files, but history should be clean.
📚 Documentation preview 📚: https://cpython-previews--114344.org.readthedocs.build/