Skip to content

gh-102797: Add more code snippets for exec_tests and eval_tests - #102798

Merged
vstinner merged 10 commits into
python:mainfrom
Eclips4:issue_102797
Jun 19, 2024
Merged

vstinner merged 10 commits into
python:mainfrom
Eclips4:issue_102797

Conversation

@Eclips4

@Eclips4 Eclips4 commented Mar 17, 2023 •

Copy link
Copy Markdown
Member

@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Mar 17, 2023

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For first review, exec test look ok. Aside from 1 addition, I think fewer comments needed. The breadth covered by a comment seems uneven (not your fault).

Comment thread Lib/test/test_ast.py Outdated
Comment thread Lib/test/test_ast.py
Comment thread Lib/test/test_ast.py Outdated
Comment thread Lib/test/test_ast.py Outdated
Comment thread Lib/test/test_ast.py Outdated
@hugovk hugovk changed the title gh-102797: Add more code snipets for exec_tests and eval_tests gh-102797: Add more code snippets for exec_tests and eval_tests Mar 18, 2023
@Eclips4
Eclips4 requested a review from terryjreedy March 19, 2023 08:34
Comment thread Lib/test/test_ast.py
# IfExp
"foo() if x else bar()",
# JoinedStr and FormattedValue
"f'{a}'",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since FormattedValue cannot be used without a JoinedStr, so separate this snippets makes no sense.

@Eclips4

Eclips4 commented Mar 20, 2023 •

Copy link
Copy Markdown
Member Author

However, single_tests look kinda strange. It says: These are compiled through "single" because of ovelarp with "eval", it just tests what can't be tested with "eval". I think, there should be more tests, and this comment is pointless.

But I don't know, should it be a separate PR?

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. More tests are usually a good thing.

@vstinner

vstinner commented Jun 7, 2024

Copy link
Copy Markdown
Member

Ah, sadly there is now a merge conflict, I cannot merge your PR.

@Eclips4

Eclips4 commented Jun 18, 2024 •

Copy link
Copy Markdown
Member Author

Oh, I didn't get a notification from this PR..
Thanks @nineteendo for resolving merge conflicts!
This PR now probably needs to be a little bit of update, I'll do it today.

@Eclips4
Eclips4 requested a review from vstinner June 18, 2024 18:54
@vstinner
vstinner merged commit b8fb369 into python:main Jun 19, 2024
@vstinner

Copy link
Copy Markdown
Member

Merged, thank you.

@Eclips4
Eclips4 deleted the issue_102797 branch June 19, 2024 09:03
mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Nineteendo <nineteendo19d0@gmail.com>
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Nineteendo <nineteendo19d0@gmail.com>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Nineteendo <nineteendo19d0@gmail.com>
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.

Implement TODO in test_ast.py

6 participants