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

Add infrastructure allowing for test cases for third-party stubs #8700

Merged
merged 11 commits into from
Sep 8, 2022

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Sep 7, 2022

This PR:

  • Moves the logic for running mypy on the test cases from tests/mypy_test.py to a separate script, tests/regr_test.py.
  • Adds the necessary logic in order to be able to have test cases for third-party stubs.
  • Moves logic common to tests/mypy_test.py and tests/regr_test.py into tests/colors.py, and renames tests/colors.py to tests/utils.py.
  • Adds a new check to tests/check_consistent.py, to enforce the use of # pyright: reportUnnecessaryTypeIgnoreComment=true comments in third-party test cases. These are essential if we want to have our tests against false-negatives work with pyright.
  • Updates the relevant documentation to account for the new test file.
  • Adds a new job to the tests.yml GitHub workflow, to run the new test in CI.
  • Adds a simple proof-of-concept test case for requests, as a regression test for requests: allow passing a list of tuples for files #7998.
  • Closes Figure out a way to have test cases for third-party stubs #8569

@srittau
Copy link
Collaborator

srittau commented Sep 7, 2022

Not reviewed yet, but the list above sounds great!

@AlexWaygood AlexWaygood marked this pull request as draft September 7, 2022 16:12
@AlexWaygood AlexWaygood marked this pull request as ready for review September 7, 2022 16:27
@AlexWaygood AlexWaygood marked this pull request as draft September 7, 2022 16:33
@AlexWaygood AlexWaygood marked this pull request as ready for review September 7, 2022 17:54
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@python python deleted a comment from github-actions bot Sep 7, 2022
@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as draft September 7, 2022 18:05
@AlexWaygood AlexWaygood marked this pull request as ready for review September 7, 2022 18:07
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member Author

Okay, this should now be ready for review! Getting this to work both locally and in CI was a bit of a wild ride.

The second commit deliberately introduces errors in stdlib test cases and third-party test cases, to show what it looks like in CI when mypy and pyright fail on those test cases. Unfortunately, it looks like if there are failures in the stdlib test cases, pyright won't then go on to check the third-party test cases. I'm not sure that's fixable without rethinking how we run pyright in CI:

The third commit fixes the errors in the stdlib test cases, so we can see what it looks like when there are only errors in the third-party test cases:

The fourth commit fixes the errors in the third-party test cases, so we're back to how it was in commit 1.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks! A few nits.

tests/check_consistent.py Outdated Show resolved Hide resolved
tests/regr_test.py Outdated Show resolved Hide resolved
tests/regr_test.py Outdated Show resolved Hide resolved
tests/regr_test.py Outdated Show resolved Hide resolved
tests/utils.py Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

LGTM, although I skipped some of the code for time constraints. One optional remark and some comment fluff.

test_cases/README.md Outdated Show resolved Hide resolved
Comment on lines +28 to +29
SUPPORTED_PLATFORMS = ["linux", "darwin", "win32"]
SUPPORTED_VERSIONS = ["3.11", "3.10", "3.9", "3.8", "3.7"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should move this into utils.py as well? Or maybe add a config.py?

Copy link
Member Author

@AlexWaygood AlexWaygood Sep 8, 2022

Choose a reason for hiding this comment

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

I think it might be better to keep this per-test. We already have one test that supports a narrower version range than this (with good reason, since we're using syntax new to 3.9 in many of our tests).

SUPPORTED_VERSIONS = ("3.11", "3.10", "3.9")

And we started testing 3.11 with mypy_test.py much earlier than we started testing it with stubtest_stdlib.py.

tests/utils.py Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Sep 8, 2022

Thanks for the reviews! I'm going to land this now, but happy to address any further feedback anybody has in follow-up PRs :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Figure out a way to have test cases for third-party stubs
3 participants