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 directories not cleaned up after test #12773

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

matthewhughes934
Copy link
Contributor

@matthewhughes934 matthewhughes934 commented Jun 18, 2024

The test_rmtree_errorhandler_reraises_error test creates a directory
that the user is unable to read under pytest's tmpdir. Once the test
session end pytest will fail to clean this up due to permissions error
and emit a warning like:

/home/user/src/pip/.nox/test-3-12/lib/python3.12/site-packages/_pytest/pathlib.py:98: PytestWarning: (rm_rf) error removing /tmp/pytest-of-user/garbage-23e907a9-3e54-40bd-89e4-d70150b10f61/test_rmtree_errorhandler_rerai0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_rmtree_errorhandler_rerai0'
  warnings.warn(

This restores behaviour from b5c1c76,
that commit mentions fixing 'failing tests' though I'm not sure which
failures that addresses and if they were related to restoring the file
permissions.

Also make consistent use of pathlib.Path methods rather than mixing
them with os. methods

@uranusjr
Copy link
Member

Failures are not related (caused by a CPython change).

@matthewhughes934
Copy link
Contributor Author

Failures are not related (caused by a CPython change).

Thanks, I was just looking into this (I see you've raised a PR to address it)

The `test_rmtree_errorhandler_reraises_error` test creates a directory
that the user is unable to read under pytest's `tmpdir`. Once the test
session end pytest will fail to clean this up due to permissions error
and emit a warning like:

    /home/user/src/pip/.nox/test-3-12/lib/python3.12/site-packages/_pytest/pathlib.py:98: PytestWarning: (rm_rf) error removing /tmp/pytest-of-user/garbage-23e907a9-3e54-40bd-89e4-d70150b10f61/test_rmtree_errorhandler_rerai0
    <class 'OSError'>: [Errno 39] Directory not empty: 'test_rmtree_errorhandler_rerai0'
      warnings.warn(

This restores behaviour from b5c1c76,
that commit mentions fixing 'failing tests' though I'm not sure which
failures that addresses and if they were related to restoring the file
permissions.

Also make consistent use of `pathlib.Path` methods rather than mixing
them with `os.` methods
@matthewhughes934
Copy link
Contributor Author

latest change is just me rebasing on main with the Windows tests fix, so checks should be all green now 🤞

@matthewhughes934 matthewhughes934 marked this pull request as ready for review June 21, 2024 08:41
@ichard26 ichard26 added the C: automation Automated checks, CI etc label Jun 22, 2024
@ichard26 ichard26 self-requested a review June 22, 2024 01:36
Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I'm glad to see these warnings disappear. Thanks :)

@ichard26 ichard26 added C: tests Testing and related things and removed C: automation Automated checks, CI etc labels Jun 23, 2024
@uranusjr uranusjr merged commit 00c75c4 into pypa:main Jun 24, 2024
31 checks passed
@matthewhughes934 matthewhughes934 deleted the fix-tests-unable-to-cleanup branch June 24, 2024 20:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bot:chronographer:provided C: tests Testing and related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants