Skip to content

Pytest do not rewrite assertions by default#117060

Closed
clee2000 wants to merge 1 commit intomainfrom
csl/pytest_no_assertion_rewriting
Closed

Pytest do not rewrite assertions by default#117060
clee2000 wants to merge 1 commit intomainfrom
csl/pytest_no_assertion_rewriting

Conversation

@clee2000
Copy link
Copy Markdown
Contributor

@clee2000 clee2000 commented Jan 9, 2024

From https://pytest.org/en/7.4.x/how-to/assert.html#advanced-assertion-introspection
pytest only rewrites test modules directly discovered by its test collection process, so asserts in supporting modules which are not themselves test modules will not be rewritten.

In CI we usually call the test file (python test_ops.py), which then calls run_test which then calls pytest.main, so the test module is already imported as __main__, so pytest does not import the test module itself and relies on the already imported module. (#95844)

However, calling pytest test_ops.py will rely on pytest to import the module, resulting in asserts being rewritten, so I add --assert=plain by default into the opts so we don't have to worry about this anymore. Another way to make pytest stop assertion rewriting in a file is to include PYTEST_DONT_REWRITE somewhere in the file.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jan 9, 2024
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Jan 9, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/117060

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit 940a2b8 with merge base 56d7a47 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Copy Markdown
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

didn't realize we had a mypy.ini already, thank you!

@clee2000 clee2000 marked this pull request as ready for review January 10, 2024 17:19
@clee2000
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 10, 2024
@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions github-actions bot deleted the csl/pytest_no_assertion_rewriting branch February 20, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants