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

Caching does not work with xdist #37

Open
GergelyKalmar opened this issue Nov 23, 2021 · 5 comments
Open

Caching does not work with xdist #37

GergelyKalmar opened this issue Nov 23, 2021 · 5 comments

Comments

@GergelyKalmar
Copy link

It seems that when running pytest-isort with pytest-xdist the cache does not get populated (.pytest_cache/.../v/isort/mtimes is empty). This means that test are re-run on all files when using distributed test execution, which is not very optimal.

Interestingly the same issue appears with pytest-pylint too (see carsongee/pytest-pylint#161, it is actually much worse there because all tests are executed on all workers due to the way the linting is done), I suppose because both plugins are based on pytest-flake8 (I haven't tested it but I think that plugin has the same issue too).

At the same time pytest-mypy works fine because it relies on the built-in cache of mypy. I'm not yet sure why does the cache not get populated with xdist, but it would be awesome if we could figure it out.

@GergelyKalmar
Copy link
Author

The problem seems to be with this line: https://github.com/stephrdev/pytest-isort/blob/master/pytest_isort/__init__.py#L77

It appears that config._isort_mtimes is an empty dictionary when using the plugin together with xdist. This is most likely because pytest_sessionfinish is run in the controller node but the part which adds the entries to _isort_mtimes runs on the workers.

I am not yet sure how to solve this issue properly. Perhaps we can send the mtimes back to the controller and save the values there, but I'm not sure how to do that.

@GergelyKalmar
Copy link
Author

We can probably do something similar to pytest-mypy: https://github.com/dbader/pytest-mypy/blob/master/src/pytest_mypy.py#L61

They're basically using pytest_configure_node to pass information from the controller to the workers, we can probably do something similar to get information back from a worker regarding mtimes. It is a pretty major re-work of the plugin to support this though I imagine.

@stephrdev
Copy link
Owner

Hey @GergelyKalmar,

thank you for your detail investigation on the issue. I think you are right. We would need to re-implement the caching similar to the MyPy plugin.

Sadly, I won't be able to tackle that before January. If you feel like you want to do it, you have my full support on reviewing and merging asap.

@GergelyKalmar
Copy link
Author

Hi @stephrdev, no worries! I've ended up building a plugin from scratch because I figured I can create an abstraction above all the plugins which rely on running something against a given source file (such as isort, pylint, pycodestyle or pydocstyle), so this way I've solved the issue with basically all plugins I use at the same time. Unfortunately that code is part of a larger private plugin suite, however, the core of the solution is to use the pytest_testnodedown hook with node.workeroutput to pass data from the workers to the controller. You can use pytest_sessionfinish to set the workeroutput (when you are on a worker node) or update the cache (when you are on the controller node).

I think the pytest-mypy plugin works differently – you will notice that it does not skip files but runs mypy against all of them. This is possible because mypy maintains its own cache so subsequent runs are usually quite fast without skipping.

@iurisilvio
Copy link
Contributor

I tried a solution to this issue and it is merged here already. #48

It is not perfect because you still has a minor race condition, but is a lot better than before.

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