Write cache for modules with errors #19820
Conversation
This comment has been minimized.
This comment has been minimized.
| if not blocker: | ||
| # There should be valid cache metadata for each module except | ||
| # for those that had an error in themselves or one of their | ||
| # dependencies. |
There was a problem hiding this comment.
Is this comment up-to-date? Don't we know have cache metadata even if there are non-blocker errors?
There was a problem hiding this comment.
Yeah, this should now read "... except for those that had a blocker error in themselves or ..."
| [file m/c.py] | ||
| import m # No error here | ||
| [rechecked m.a, m.b] | ||
| [rechecked] |
There was a problem hiding this comment.
Do we have a test where errors are reported based on cache from multiple source files? I think this would be useful to have, to make sure the error ordering is as expected etc. (Though maybe we already have this tested somewhere else.)
There was a problem hiding this comment.
FWIW I think we have a lot of incremental tests already :-) But if I will not find anything after a quick look I will add one more.
| return None | ||
| is_errors = self.transitive_error | ||
| if is_errors: | ||
| delete_cache(self.id, self.path, self.manager) |
There was a problem hiding this comment.
What if there are blockers? Would we then need to delete the cache file? I wonder if we have a test for this, i.e. blocker is introduced, and the following incremental run still has the same blocker.
There was a problem hiding this comment.
Blocker are always exceptions in _build() caught by build(), so we will not even get to the point where we write cache. If there was an existing one, it is fine. It will be either discarded on the next run as well (because it was discarded on this run, as we were checking the file for some reason), or actually used if a user does an "undo" in the file. I think there is never a reason to delete an existing cache file.
However, surprisingly I can't find an existing test for this.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
This is required for #933
Here I use a very simple-minded approach, errors are serialized simply as a list of strings. In near future I may switch to serializing
ErrorInfos (as this has some other benefits). Note that many tests have[stale ...]checks updated because previously modules with errors were not included in the list. I double-checked each test that the new values are correct.Note we still don't write cache if there were blockers in an SCC (like a syntax error).