-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add extensive tests for incremental type checking #1349
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
Comments
FWIW I think it's fine to release 0.3.2 with this off by default -- that way people can play with it and report their findings to us, which will help us squeeze out the bugs, understand use cases, and expand the test suite. |
We also need specific tests for the interaction between --incremental and --silent-imports (see #1383). |
One easy test to add would be running the equivalent of |
At the very least that should be a standard check during QA for a release...
|
A different approach would be to take an internal (large) codebase in git that is known to pass the type checker cleanly, randomly pick e.g. a sequence of 30 revisions out of the last 3000 (not necessarily in chronological order!), and run mypy in incremental mode across this sequence. Historically most esoteric incremental bugs have been reported by users who made some random big jump in their git repo between incremental runs (e.g. pulling a week's worth of changes or switching branches). The existing test script misc/incremental_checker.py could easily be adapted to do this (it currently does a similar thing but only checks the 30 latest consecutive revisions, which is much less likely to trip over some of the more esoteric problems). For a repo where a typical run takes 2-3 minutes such an experiment could run in 1-2 hours. |
Maybe mypy should be run outside incremental mode on each commit, too? That way, any errors in the code due to WIP commits/un-squashed PRs wouldn't cause the whole thing to randomly fail. |
Good idea, though it could make things much slower. Our internal codebase doesn't have commits that fail mypy, but sometimes a newer mypy version complains about older commits, so it's probably still a good idea to add this. Maybe only when the incremental run fails? |
I think the script modified in #2583 already does that? iirc when I was writing the script, mypy would sometimes throw errors even during the last 10-30 commits due to recent changes/fixes, so I ended up needing to do that anyways out of necessity. |
Yup, it does. Thanks for making that script easy to extend!
…--Guido (mobile)
On Dec 15, 2016 1:43 PM, "Michael Lee" ***@***.***> wrote:
Maybe mypy should be run outside incremental mode on each commit, too?
That way, any errors in the code due to WIP commits/un-squashed PRs
wouldn't cause the whole thing to randomly fail.
I think the script modified in #2583
<#2583> already does that?
iirc when I was writing the script, mypy would sometimes throw errors even
during the last 10-30 commits due to recent changes/fixes, so I ended up
needing to do that anyways out of necessity.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1349 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACwrMhTRMxaxCAHachQH9RuC5P4qTIx3ks5rIbSJgaJpZM4IDOCU>
.
|
I'm going to declare this done. It's been pretty satisfying to see this run. |
We need more tests before turning incremental mode on by default (+ more manual testing).
The text was updated successfully, but these errors were encountered: