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-123361: Split test_dis.py into a directory #123362

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Aug 26, 2024

Both invocation styles pass:

» ./python.exe Lib/test/test_dis/test_dis.py
...........................................................................................................................................
----------------------------------------------------------------------
Ran 139 tests in 0.395s

OK

@iritkatriel
Copy link
Member

I'm not sure this is what we want to do. It would be better to get rid of these test altogether. It's not clear what they are testing. See #90916

@sobolevn
Copy link
Member Author

It would be better to get rid of these test altogether.

Yes, I've mentioned this in the issue. I still think that this is the step in the right direction, because it will be easier to work with test_dis this way and to remove / refactor snippet tests in the future.

@erlend-aasland erlend-aasland removed their request for review August 28, 2024 07:04
@vstinner
Copy link
Member

vstinner commented Sep 2, 2024

It would be better to get rid of these test altogether

Which part of test_dis? The whole file?

@sobolevn
Copy link
Member Author

sobolevn commented Sep 2, 2024

@vstinner probably @iritkatriel means new snippet.py file and tests from it.

@iritkatriel
Copy link
Member

It would be better to get rid of these test altogether

Which part of test_dis? The whole file?

The whole file needs to be rewritten probably. We need to rethink what it means to test the dis module.

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.

3 participants