pytest-mypy-plugins icon indicating copy to clipboard operation
pytest-mypy-plugins copied to clipboard

Broken mypy cache?

Open zero323 opened this issue 3 years ago • 2 comments

I am trying to investigate some issues related to caching behavior. When testing project with complex dependencies, I see serious performance degradation (roughly 20 fold on just 36 tests) compared to using mypy.test.testcheckTypeCheckSuite directly.

I thought the issue was a simple logic mistake (#82), but it seems it might be actually with find_dependent_paths

https://github.com/typeddjango/pytest-mypy-plugins/blob/a2d4adde12b0024e62f2e1661fd0dd5abb4f9191/pytest_mypy_plugins/item.py#L187

Since it uses at least main.py it includes all kinds of packages using main as a name (not necessarily as a module name, could be even an argument), for example

['/tmp/.mypy_cache/3.9/pdb',
 '/tmp/.mypy_cache/3.9/unittest/main',
 '/tmp/.mypy_cache/3.9/unittest/__init__',
 '/tmp/.mypy_cache/3.9/_pytest/pytester',
 '/tmp/.mypy_cache/3.9/_pytest/config/__init__',
 '/tmp/.mypy_cache/3.9/pytest/__init__',
 '/tmp/.mypy_cache/3.9/asyncio/runners']

This seems to escalate (in my case, to numpy annotations, for reason yet to be determined), and break caching in general.

Possibly related to #37

Originally posted by @zero323 in https://github.com/typeddjango/pytest-mypy-plugins/issues/82#issuecomment-945904250

zero323 avatar Oct 18 '21 15:10 zero323

I guess that the real question is, why this part might be needed:

https://github.com/typeddjango/pytest-mypy-plugins/blob/a2d4adde12b0024e62f2e1661fd0dd5abb4f9191/pytest_mypy_plugins/item.py#L302-L304

Are there any cases, where test case cache can leak to files not identified by simply iterating over self.files?

zero323 avatar Oct 18 '21 16:10 zero323

Just thinking out loud:

  • Current find_dependent_paths logic should be dropped completely. It cannot be relied on. If anything, we might want to look at cross_refs (see below).

  • Possibly, the whole dependant block is unnecessary (maybe @mkurnikov could chip in here?). To my limited understanding, the only possible indicator of dependence would be cross_ref, and I don't see how it could for an external module.

  • Is post-test cleanup really necessary? Meta files should contain all relevant information required to identify out-of-date cache and we don't really do cleanup during manual checks, do we?

    Alternatively, we could skip this step unless cache_fine_grained was used and we have detailed dependency information.

  • If desired at all, cleaning should be probably applied recursively (if some module depends on one that is to be removed, all its defendants should be removed as well).

  • Is there a deeper problem here? What if cache dir was reused (#84)? Are we likely to end up with race conditions? Could we place individual test items in their own namespaces, i.e creating packages with f"{normalize(test_file_name)}/{normalize(test_case_name)}".

zero323 avatar Oct 19 '21 12:10 zero323