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

Fix tmp_path regression introduced in 7.3.0 #10911

Merged
merged 6 commits into from
Apr 14, 2023

Conversation

nicoddemus
Copy link
Member

The problem is that we would loop over all directories of the basetemp directory searching for dead symlinks, for each test, which would compound over the test session run.

Doing the cleanup just once, at the end of the session, fixes the problem.

Fix #10896

The problem is that we would loop over all directories of the basetemp directory searching
for dead symlinks, for each test, which would compound over the test session run.

Doing the cleanup just once, at the end of the session, fixes the problem.

Fix pytest-dev#10896
@nicoddemus
Copy link
Member Author

To test this, I wrote this file:

import pytest

DATA = "0" * 1 * 1024 * 1024


@pytest.mark.parametrize("i", range(2000))
def test1(tmp_path, i):
    tmp_path.joinpath("hello.txt").write_text(DATA)

In pytest-7.3.0, I get:

2000 passed in 314.96s (0:05:14)

(You can see each test taking longer and longer).

In this branch I get:

2000 passed in 15.21s

I couldn't think of a test for this, given the regression is circumstantial.

@The-Compiler
Copy link
Member

@nicoddemus Any chance you could push the tags to your repo so I can easily test this? Right now I get:

ERROR: Cannot install -r /home/florian/proj/qutebrowser/git/misc/requirements/requirements-tests.txt (line 33) and pytest 6.0.1.dev2203+g2d01dbd0 (from git+https://github.com/nicoddemus/pytest.git@tmp-path-regression) because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested pytest 6.0.1.dev2203+g2d01dbd0 (from git+https://github.com/nicoddemus/pytest.git@tmp-path-regression)
    pytest-bdd 6.1.1 depends on pytest>=6.2.0

Something like this should do:

git fetch upstream --tags
git push origin --tags

@The-Compiler
Copy link
Member

I got it to work via SETUPTOOLS_SCM_PRETEND_VERSION=7.3.1 🤷

Seems to work fine, thanks for the investigation and fix! Down from 1h 5min to 24 minutes again now.

@nicoddemus
Copy link
Member Author

Ahh sorry, just saw it now. Anyway I pushed the tags, glad you could work around that.

@nicoddemus nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Apr 14, 2023
@nicoddemus nicoddemus merged commit d380771 into pytest-dev:main Apr 14, 2023
22 of 23 checks passed
@nicoddemus nicoddemus deleted the tmp-path-regression branch April 14, 2023 16:24
@nicoddemus nicoddemus added backport 7.3.x and removed needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch labels Apr 14, 2023
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

Successfully merging this pull request may close these issues.

Major pytest 7.3.0 performance regression on Windows
4 participants