Skip to content

gh-115596: Fix ProgramPriorityTests in test_os permanently changing the process priority#115610

Merged
serhiy-storchaka merged 6 commits into
python:mainfrom
brianschubert:fix-test-os-setpriority
Feb 17, 2024
Merged

gh-115596: Fix ProgramPriorityTests in test_os permanently changing the process priority#115610
serhiy-storchaka merged 6 commits into
python:mainfrom
brianschubert:fix-test-os-setpriority

Conversation

@brianschubert

@brianschubert brianschubert commented Feb 17, 2024

Copy link
Copy Markdown
Contributor

Fixes #115596

Runs the priority setting logic from ProgramPriorityTests.test_set_get_priority in a subprocess to prevent the priority change from taking permanent effect.

@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Feb 17, 2024

@serhiy-storchaka serhiy-storchaka 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.

When the test fails, we only see something like:

subprocess.CalledProcessError: Command '['/home/serhiy/py/cpython/python', '-c', 'if 1:\n        import os\n        os.setpriority(os.PRIO_PROCESS, os.getpid(), 0 + 1)\n        print(os.getpriority(os.PRIO_PROCESS, os.getpid()))\n        ']' returned non-zero exit status 1.

Which does not give any information about the cause of the failure. You can use assert_python_ok() which shows the stdout and the stderr of the failed process.

Comment thread Lib/test/test_os.py Outdated

@serhiy-storchaka serhiy-storchaka 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.

Pure test PRs usually do not need a NEWS entry, but this bug potentially had a large scope.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Feb 17, 2024
@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) February 17, 2024 16:06
@serhiy-storchaka serhiy-storchaka merged commit 90dd653 into python:main Feb 17, 2024
@miss-islington-app

Copy link
Copy Markdown

Thanks @brianschubert for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 17, 2024
…ging the process priority (pythonGH-115610)

(cherry picked from commit 90dd653)

Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
@bedevere-app

bedevere-app Bot commented Feb 17, 2024

Copy link
Copy Markdown

GH-115616 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Feb 17, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 17, 2024
…ging the process priority (pythonGH-115610)

(cherry picked from commit 90dd653)

Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
@bedevere-app

bedevere-app Bot commented Feb 17, 2024

Copy link
Copy Markdown

GH-115617 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Feb 17, 2024
serhiy-storchaka pushed a commit that referenced this pull request Feb 17, 2024
…nging the process priority (GH-115610) (GH-115616)

(cherry picked from commit 90dd653)

Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Feb 17, 2024
…nging the process priority (GH-115610) (GH-115617)

(cherry picked from commit 90dd653)

Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_os permanently change the process priority

2 participants