mypy_test.py: Move type-checking of our tests and scripts into a different test
#8587
Conversation
There was a problem hiding this comment.
Thanks for cleaning this up.
I have some minor feedback:
- Calling a workflow
mypy_selfcheckis very confusing, especially given that typeshed used to have a test called mypy self check where we'd run mypy over mypy using custom typeshed. - It feels like there are a number of uses of the word "test" floating around. Maybe "typecheck-typeshed-code" is clearer than "test-the-tests" and better covers the fact that we type check scripts?
- This isn't worth 9 CI jobs; the multiple Python versions is overkill. Let's just do 3.9. (Tbh, multiple platforms is also overkill, maybe just make it
--platform win32and call it a day)
Yeah, I had no idea what to call this workflow :) your suggestion is good! |
|
Thanks for the review! :) I'll leave this open for a little bit to see if anybody else has any thoughts. |
| if platform == "win32": | ||
| command.extend(["--exclude", "tests/pytype_test.py"]) | ||
| else: | ||
| command.append("--ignore-missing-imports") |
There was a problem hiding this comment.
Side note: We should probably add the types for the imports we use to tests-requirements. (In a different PR.)
There was a problem hiding this comment.
Or maybe script-requirements/stubsabot-requirements? (Most contributors aren't going to need to run stubsabot manually, after all :)
There was a problem hiding this comment.
I think it's easier to just put all the requirements into one file. It won't hurt much to install these extra requirements, but it's a bit lower of a mental burden for when you need those.
There was a problem hiding this comment.
Looking at it again, it seems as though stubsabot's additional requirements are pretty minimal anyway, so it probably doesn't hurt -- you're right!
|
I'll merge this now, but happy to address any further feedback anybody has in a followup PR! |
As requested in #8139 (comment)