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

Improvements around stubtest #3728

Closed
hauntsaninja opened this issue Feb 5, 2020 · 9 comments
Closed

Improvements around stubtest #3728

hauntsaninja opened this issue Feb 5, 2020 · 9 comments

Comments

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 5, 2020

Creating this issue to discuss and document areas of improvement from using stubtest. Note when I use "in CI" below, I mean as in current draft pull request #3727

  • Fixing existing errors and burning down the whitelists. Some notes:
    • Watch out for unnecessary branches. inspect.signature sometimes throws, in which case stubtest doesn't analyse further. However, the functions it fails for differ from python version to python version. If you're not sure whether you need to branch on version, run stubtest without the branch (the documentation is also helpful).
    • Similarly, as per discussion in How much do we care about positional-only args? #3693, in CI, we don't check positional-only accuracy for versions other than py38, so don't branch for that.
    • For whitelist entries that shouldn't be fixed, it might be worth adding comments to save effort and prevent others from attempting the fix (e.g., cmath.log). For avoidable stubtest false positives, please tag me!
    • To see existing errors, check the links in Better testing of stubs #754 (comment) (I've kept them up-to-date) — or run stubtest yourself!
  • In CI, we run stubtest with --ignore-missing-stub. Running stubtest without this flag is an easy way to find missing stubs to add.
  • In CI, we only run on Linux. Running stubtest on different platforms could be a good way of fixing platform differences/availability (be careful about --ignore-missing-stub).
  • In CI, we only check stdlib. Running stubtest against third party stubs would be good (potentially in CI as well).
  • Note that new Python releases will require reworking of the whitelists. This can apply to point releases too (which would break CI), e.g, when the backport of https://bugs.python.org/issue39493 gets released and Travis updates.
    • Stubtest can be useful for helping update typeshed, see details:

Changes:

python3.8 stubtest.py --custom-typeshed-dir ~/dev/typeshed --ignore-missing-stub --check-typeshed --generate-whitelist > py38wl.txt
python3.9 stubtest.py --custom-typeshed-dir ~/dev/typeshed --ignore-missing-stub --check-typeshed --whitelist py38wl.txt

Additions:

python3.9 stubtest.py --custom-typeshed-dir ~/dev/typeshed --ignore-missing-stub --check-typeshed --generate-whitelist > py39wl.txt
python3.8 stubtest.py --custom-typeshed-dir ~/dev/typeshed --check-typeshed --generate-whitelist > py38wl2.txt
python3.9 stubtest.py --custom-typeshed-dir ~/dev/typeshed --check-typeshed --concise --whitelist py38wl2.txt --whitelist py39wl.txt
@CraftSpider
Copy link
Contributor

It seems that stubtest chokes on 'private' names, like __dump. It expects there to be a name literally called __dump instead of _[classname]__dump on the runtime type. While that kind of method is implicitly undocumented, I think it's still worth it to handle them correctly

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Mar 7, 2020

Thanks for reporting! Just added python/mypy@f1a1b66 to an already open pull request (python/mypy#8502) that should fix this

@CraftSpider
Copy link
Contributor

Is there a good way to whitelist a module on only certain OSes? Stubtest complains for a bunch of modules that don't exist on windows, while I would like to start it running on things like winreg and msvcrt when it's run on windows

@hauntsaninja
Copy link
Collaborator Author

If you run tests/stubtest_test.py on Windows, it should already run on winreg and msvcrt. If you don't see any errors regarding those modules, maybe attempt to run stubtest directly on them? E.g. python3.8 -m mypy.stubtest --custom-typeshed-dir ~/wherever/typeshed winreg

For whitelisting additional errors on Windows, maybe try modifying tests/stubtest_test.py to pass in an extra whitelist based on sys.platform? https://github.com/python/typeshed/blob/master/tests/stubtest_test.py#L33

If you start running into issues with unused whitelist entries, there are two tools at your disposal:

Are you working your way towards running stubtest on Windows in CI, or are you just looking to make improvements based on local runs?

@CraftSpider
Copy link
Contributor

Currently I'm just looking at local improvements. While I wouldn't be against working towards windows CI, I can't dedicate to that cause right now. Maybe once I've finished my current main project, stdlib module/package existence.

@srittau
Copy link
Collaborator

srittau commented May 28, 2020

@hauntsaninja Currently stubtest fails if a whitelist entry has become obsolete due to a PR. I think this is a bit problematic, since its easily missed when providing an easy fix, and makes it a bit harder to contribute them. Do you think something like the following would make sense?

  • Don't fail CI if there are extraneous entries on the whitelist.
  • Run a GitHub Action daily (or maybe just weekly to keep the noise low) that checks for such entries and auto-generates a PR to remove them.

@hauntsaninja
Copy link
Collaborator Author

Yeah, that makes sense! stubtest already supports --ignore-unused-whitelist, so should be pretty doable.

@srittau
Copy link
Collaborator

srittau commented May 28, 2020

I will see whether I can try something out over the weekend.

srittau added a commit to srittau/typeshed that referenced this issue Jun 2, 2020
This is currently a GitHub workflow that runs daily and lists all unused
whitelist entries found by running stubtest against current Python
versions on Linux and Windows. The workflow run will succeed if there
are no such entries, and fail otherwise.

In a second step, this should collate the output of the various runs and
create a PR to remove the entries. In that case, the workflow should
probably only run weekly or even monthly to keep the noise down.

Cf. python#3728
srittau added a commit that referenced this issue Jun 3, 2020
* Find unused stubtest whitelist entries

This is currently a GitHub workflow that runs daily and lists all unused
whitelist entries found by running stubtest against current Python
versions on Linux and Windows. The workflow run will succeed if there
are no such entries, and fail otherwise.

In a second step, this should collate the output of the various runs and
create a PR to remove the entries. In that case, the workflow should
probably only run weekly or even monthly to keep the noise down.

Cf. #3728
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this issue Jun 26, 2020
* Find unused stubtest whitelist entries

This is currently a GitHub workflow that runs daily and lists all unused
whitelist entries found by running stubtest against current Python
versions on Linux and Windows. The workflow run will succeed if there
are no such entries, and fail otherwise.

In a second step, this should collate the output of the various runs and
create a PR to remove the entries. In that case, the workflow should
probably only run weekly or even monthly to keep the noise down.

Cf. python#3728
@hauntsaninja
Copy link
Collaborator Author

Closing this. I think the only action item here is to run stubtest on third party libraries, which I'd be more inclined to take on once we have modular typeshed.

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

No branches or pull requests

3 participants