-
Notifications
You must be signed in to change notification settings - Fork 658
Skipping test_permission_error
if runner is root.
#502
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the skip condition on the test_permission_error test to also skip execution when the tests are running as root.
- Updated skipif condition in the test to include "os.getuid() == 0"
- Adjusted the reason message to reflect the change
@@ -99,7 +99,8 @@ async def test_missing_file_error(self, temp_file: Path): | |||
await resource.read() | |||
|
|||
@pytest.mark.skipif( | |||
os.name == "nt", reason="File permissions behave differently on Windows" | |||
os.name == "nt" or os.getuid() == 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using hasattr(os, 'getuid') or similar guards to ensure os.getuid() is only accessed on platforms where it is defined, improving code clarity and maintainability.
os.name == "nt" or os.getuid() == 0, | |
os.name == "nt" or (hasattr(os, 'getuid') and os.getuid() == 0), |
Copilot uses AI. Check for mistakes.
All tests pass now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issue #501 by ensuring that the test_permission_error test is skipped when running on Windows or as the root user.
- Updated skip condition in test_permission_error to include a check for os.getuid() being 0.
- Modified the reason message to reflect the updated condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for posterity @ZiadAmerr do you mind sharing your motivation for running tests as root? this seems fine to me though
Hi @zzstoatzz. It was actually a complete coincidence. I had pulled the repo and ran the tests after I was using root for other stuff. Mere coincidence. |
This fixes:
test_permission_error_debug
failing when running as root. #501I have added a check
or os.getuid() == 0
to the skipping condition of the test to make sure it is skipped when running as root.