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(pytest): Adding workaround to avoid systemic CI errors #85996

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

IanWoodard
Copy link
Member

We’ve observed systemic failures in CI, causing the entire test suite to fail due to access being denied. This issue arises from our test fixture, which ensures no file descriptors are leaked between tests. The problem occurs when a file descriptor is found that we are unable to access, triggering a PermissionError.
The underlying cause stems from the logic used by psutil, which performs more checks than necessary. Specifically, it attempts to verify whether the path is a valid file using os.stat(), leading to the access issues, as shown in the error below:

______ ERROR at setup of QuantizeTimeTest.test_quantizes_with_rounding_up ______
.venv/lib/python3.13/site-packages/psutil/_pslinux.py:1643: in wrapper
    return fun(self, *args, **kwargs)
.venv/lib/python3.13/site-packages/psutil/_pslinux.py:2227: in open_files
    if path.startswith('/') and isfile_strict(path):
.venv/lib/python3.13/site-packages/psutil/_common.py:526: in isfile_strict
    st = os.stat(path)
E   PermissionError: [Errno 13] Permission denied: '/root/.dotnet/corefx/cryptography/x509stores/my/226A849E790F34CD326D337D1FB66D8420F6C32B.pfx'

To mitigate this issue, we’ve simplified the implementation for Linux by using a more direct approach to retrieve open file descriptors via the /proc file system, which avoids triggering os.stat() and the associated permission error. Since the issue appears to be isolated to CI (which runs on Linux), we’ve limited this fix to Linux.

@IanWoodard IanWoodard requested review from asottile-sentry and a team February 26, 2025 22:46
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tests/conftest.py 88.88% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #85996       +/-   ##
===========================================
+ Coverage   46.67%   87.89%   +41.21%     
===========================================
  Files        9686     9700       +14     
  Lines      549231   549998      +767     
  Branches    21371    21371               
===========================================
+ Hits       256331   483398   +227067     
+ Misses     292585    66285   -226300     
  Partials      315      315               

Copy link
Member

@asottile-sentry asottile-sentry left a comment

Choose a reason for hiding this comment

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

@IanWoodard IanWoodard merged commit d6caf87 into master Feb 27, 2025
45 of 46 checks passed
@IanWoodard IanWoodard deleted the iw/unclosed-files-fixture-workaround branch February 27, 2025 19:29
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.

2 participants