GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-102797: Add more code snippets for exec_tests and eval_tests by Eclips4 · Pull Request #102798 · python/cpython

Conversation

@Eclips4

@Eclips4 Eclips4 commented Mar 17, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Resolves #102797

  • Issue: Implement TODO in test_ast.py #102797

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Eclips4
Add more code snipets for exec_tests and eval_tests
759b32f
@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Mar 17, 2023
@bedevere-bot bedevere-bot mentioned this pull request Mar 17, 2023
Implement TODO in test_ast.py #102797
Closed
@bedevere-bot bedevere-bot added the awaiting review label Mar 17, 2023
@carljm carljm added the skip news label Mar 17, 2023
terryjreedy
terryjreedy reviewed Mar 18, 2023
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

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.

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@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 and others added 4 commits March 18, 2023 16:00
@Eclips4 @terryjreedy
Remove comment about function without argument
7c6bf80 
Co-authored-by: Terry Jan Reedy <[email protected]>
@Eclips4 @terryjreedy
Merge two comments about yield into one comment
557e4f0 
Co-authored-by: Terry Jan Reedy <[email protected]>
@Eclips4
Move all if under one comment
0d25ba1
@Eclips4
Add test for raise when Exception instance is not created. Remove poi…
acfd433 
…ntless comment.
@Eclips4
Eclips4 requested a review from terryjreedy March 19, 2023 08:34
@Eclips4
Add snippets for JoinderStr and FormattedValue
3af8b9e
Eclips4
Eclips4 commented Mar 20, 2023
View reviewed changes
Comment thread Lib/test/test_ast.py
# IfExp
"foo() if x else bar()",
# JoinedStr and FormattedValue
"f'{a}'",

@Eclips4 Eclips4 Mar 20, 2023

Copy link
Copy Markdown
Member Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Eclips4

Eclips4 commented Mar 20, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Merge branch 'main' into issue_102797
c329846
vstinner
vstinner approved these changes Jun 7, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM. More tests are usually a good thing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jun 7, 2024
@vstinner

vstinner commented Jun 7, 2024

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
nineteendo and others added 2 commits June 16, 2024 13:28
@nineteendo
Merge branch 'main' into issue_102797
4e34363
@Eclips4
Merge pull request #1 from nineteendo/issue_102797
98826a9 
Fix merge conflicts for Issue 102797
@Eclips4

Eclips4 commented Jun 18, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.
nineteendo reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Small update
4280eb7
@Eclips4
Eclips4 requested a review from vstinner June 18, 2024 18:54
@vstinner
vstinner merged commit b8fb369 into python:main Jun 19, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 19, 2024
@vstinner

vstinner commented Jun 19, 2024

Copy link
Copy Markdown
Member

Merged, thank you.
Eclips4 reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@Eclips4 @terryjreedy
@nineteendo @mrahtz
pythongh-102797: Add more code snippets in test_ast (python#102798)
dd43ccd 
Co-authored-by: Terry Jan Reedy <[email protected]>
Co-authored-by: Nineteendo <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@Eclips4 @terryjreedy
@nineteendo @estyxx
pythongh-102797: Add more code snippets in test_ast (python#102798)
799a646 
Co-authored-by: Terry Jan Reedy <[email protected]>
Co-authored-by: Nineteendo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy Awaiting requested review from terryjreedy
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement TODO in test_ast.py

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@Eclips4 @vstinner @terryjreedy @carljm @bedevere-bot @nineteendo
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.