Visitar URL original
move root checks to test.support by kulikjak · Pull Request #146195 · python/cpython · GitHub
Skip to content

move root checks to test.support - #146195

Merged
vstinner merged 5 commits into
python:mainfrom
kulikjak:tests-root-improvements
Apr 2, 2026
Merged

move root checks to test.support#146195
vstinner merged 5 commits into
python:mainfrom
kulikjak:tests-root-improvements

Conversation

@kulikjak

@kulikjak kulikjak commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Hi, when looking into unrelated changes, I realized that the check for root privilege is duplicated on several places and can be moved to test.support.

This change also improves test_ownership_after_flush because without root, it skips it before running potentially slow pwd.getpwall.

I am unsure whether this is trivial enough to not require an issue - I am happy to create one if that is not the case.

@kulikjak
kulikjak requested review from a team and barneygale as code owners March 20, 2026 08:27
vstinner
vstinner reviewed Mar 27, 2026
View reviewed changes
Comment thread Lib/test/support/__init__.py Outdated


_ROOT_IN_POSIX = hasattr(os, 'geteuid') and os.geteuid() == 0
requires_root = unittest.skipUnless(_ROOT_IN_POSIX, "test needs root privilege")

@vstinner vstinner Mar 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you rename these decorators to requires_root_user and requires_non_root_user? At the first look, I wasn't sure what "root" was about. Root window? Root what? :-) Or maybe requires_root_privilege?

@kulikjak kulikjak Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, you are right - I guess there are several roots :)

vstinner
vstinner approved these changes Mar 31, 2026
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vstinner
vstinner enabled auto-merge (squash) March 31, 2026 09:47
@vstinner

vstinner commented Apr 1, 2026

Copy link
Copy Markdown
Member

"Tests / All required checks pass" failed with:

Error: The template is not valid. .github/workflows/build.yml (Line: 687, Col: 24): Error reading JToken from JsonReader. Path '', line 0, position 0.

Let me click on [Update branch] to try to fix it.

@vstinner
vstinner merged commit 86b8617 into python:main Apr 2, 2026
48 checks passed
@vstinner

vstinner commented Apr 2, 2026

Copy link
Copy Markdown
Member

Merged, thanks for tests enhancements.

@bedevere-bot

bedevere-bot commented Apr 2, 2026

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit 86b8617.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/345/builds/13827) and take a look at the build logs.
  4. Check if the failure is related to this commit (86b8617) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/345/builds/13827

Failed tests:

  • test_pathlib

Failed subtests:

  • test_copy_dir_no_read_permission - test.test_pathlib.test_pathlib.PathSubclassTest.test_copy_dir_no_read_permission
  • test_copy_dir_no_read_permission - test.test_pathlib.test_pathlib.PosixPathTest.test_copy_dir_no_read_permission
  • test_copy_dir_no_read_permission - test.test_pathlib.test_pathlib.PathTest.test_copy_dir_no_read_permission

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_pathlib/test_pathlib.py", line 1558, in test_copy_dir_no_read_permission
    self.assertRaises(PermissionError, source.copy, target)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: PermissionError not raised by copy

@encukou

encukou commented Apr 3, 2026

Copy link
Copy Markdown
Member

Will you be able to fix the failure today?
If not, let's revert so the buildbot can find other issues.

@kulikjak

kulikjak commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi, sorry about that. I am looking into it and will try to fix it ASAP.

@kulikjak kulikjak mentioned this pull request Apr 3, 2026
Merged
@kulikjak

kulikjak commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, it was a silly little mistake - #148028 should fix it.

encukou pushed a commit that referenced this pull request Apr 3, 2026
@kulikjak
test_copy_dir_no_read_permission requires non root user (GH-148028)
3908593 
This fixes a bug introduced in commit 86b8617
(GH-146195).
@encukou

encukou commented Apr 3, 2026

Copy link
Copy Markdown
Member

Yea, I posted before looking at the code, and before coffee :)
Thank you for the fix!

@vstinner

vstinner commented Apr 3, 2026

Copy link
Copy Markdown
Member

Oops, I missed that in the review. Hopefully, we have a buildbot running as root (AMD64 Debian root 3.x) which catched the regression. Thanks for fixing the regression.

@pablogsal pablogsal mentioned this pull request Apr 4, 2026
Merged
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@kulikjak @ljfp
Move root user checks to test.support (python#146195)
db4a2f3
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@kulikjak @ljfp
test_copy_dir_no_read_permission requires non root user (pythonGH-148028
6b9e8b0 
)

This fixes a bug introduced in commit 86b8617
(pythonGH-146195).
encukou added a commit to miss-islington/cpython that referenced this pull request Sep 30, 2026
@encukou
Add root user checks to test.support
fb187f1 
This partially backports commit 86b8617
(pythonGH-146195); making existing tests use the helper is omitted.
encukou added a commit to miss-islington/cpython that referenced this pull request Sep 30, 2026
@encukou
Add root user checks to test.support
8da0b0a 
This partially backports commit 86b8617
(pythonGH-146195); making existing tests use the helper is omitted.
Yhg1s pushed a commit that referenced this pull request Sep 30, 2026
@encukou @StanFromIreland @vstinner
[3.12] gh-157579: Fix race condition in the cleanup of tempfile.Tempo…
458e713 
…raryDirectory (GH-157580) (#158489)

* gh-157579: Fix race condition in the cleanup of `tempfile.TemporaryDirectory` (GH-157580)
(cherry picked from commit 5c20517)

* Add root user checks to test.support

This partially backports commit 86b8617
(GH-146195); making existing tests use the helper is omitted.

* gh-134993: Add os.lstat() to os.supports_dir_fd (#135188)

Co-authored-by: Stan Ulbrych <stan@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@barneygale barneygale Awaiting requested review from barneygale barneygale is a code owner

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kulikjak @vstinner @bedevere-bot @encukou