Skip to content

gh-123165: correct tests for dis.dis(func, show_positions=True) #123220

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 2 commits into from
Aug 22, 2024

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Aug 22, 2024

Mark suggested simplifying the faking of positions information via ast manipulation so here it is.

cc @iritkatriel @markshannon

@iritkatriel
Copy link
Member

Note: if someone knows a way to include partial positions information as well as no position information (but only for a single instruction, and not for the entire code object otherwise we cannot test the -- rendering) by changing AST nodes only, please tell me.

The same way that dis tests '--' for line numbers should work. It inputs python code that gets compiled into something that needs '--'.

@picnixz
Copy link
Member Author

picnixz commented Aug 22, 2024

It inputs python code that gets compiled into something that needs '--'.

Mmh, yes it was dumb of me. I'll try this approach now.

@picnixz picnixz requested a review from iritkatriel August 22, 2024 09:58
@picnixz
Copy link
Member Author

picnixz commented Aug 22, 2024

The tests are a bit longer (in length) but at least I don't need some magical construction that I would forget about tomorrow :D Thank you all for the tips.

@picnixz picnixz requested a review from markshannon August 22, 2024 09:59
Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

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

LGTM

@iritkatriel iritkatriel merged commit 31acc4d into python:main Aug 22, 2024
32 checks passed
@picnixz picnixz deleted the simplify-fake-positions branch August 22, 2024 13:31
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.

2 participants