Skip to content

gh-118422: Fix run_fileexflags() test #118429

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 30, 2024
Merged

gh-118422: Fix run_fileexflags() test #118429

merged 3 commits into from
Apr 30, 2024

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 30, 2024 •
edited
Loading

Don't test undefined fileno() behavior on a closed file, but use fstat() as a reliable test if the file was closed or not.

@bedevere-app bedevere-app bot mentioned this pull request Apr 30, 2024
Closed
@vstinner
pythongh-118422: Fix run_fileexflags() test
4cba722 
Don't test undefined fileno() behavior on a closed file, but use
fstat() as a reliable test if the file was closed or not.
@serhiy-storchaka
Copy link
Member

!buildbot wasi

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 4cba722 🤖

The command will test the builders whose names match following regular expression: wasi

The builders matched are:

  • wasm32-wasi PR
  • wasm32 WASI 8Core PR
  • wasm32-wasi Non-Debug PR

serhiy-storchaka
serhiy-storchaka approved these changes Apr 30, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Thank you, it LGTM if tests are passed on all platforms.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 only security fixes tests Tests in the Lib/test dir skip news labels Apr 30, 2024
@vstinner
Copy link
Member Author

@serhiy-storchaka: I added _Py_IsValidFD() helper function to fix Windows. Do you want to review the updated PR?

@vstinner
Copy link
Member Author

Oh, the test now fails on WASI with:

test_run_fileexflags (test.test_capi.test_run.CAPITest.test_run_fileexflags) ...

Error: failed to run main module `python.wasm`

Caused by:
    0: failed to invoke command default
    1: error while executing at wasm backtrace:
           0: 0x56b06e - <unknown>!fclose
           1: 0x47526e - <unknown>!run_fileexflags

@serhiy-storchaka
Copy link
Member

Initially the check was skipped only on WASI (#117982). Then I made it only performing on Linux/Windows/macOS (#118230), because it failed on Oracle Solaris, SmartOS and OpenBSD (#117982 (comment)).

@vstinner vstinner merged commit e93c39b into python:main Apr 30, 2024
34 checks passed
@vstinner vstinner deleted the run branch April 30, 2024 20:32
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington-app
Copy link

Sorry, @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e93c39b47ea623dfaf61f80775ad4747b163efe5 3.12

SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@vstinner @SonicField
pythongh-118422: Fix run_fileexflags() test (python#118429)
1c20fc5 
Don't test the undefined behavior of fileno()
on a closed file, but use fstat() as a reliable
test if the file was closed or not.
@hugovk
Copy link
Member

hugovk commented Apr 8, 2025

3.12 is now security-only, so the backport is no longer needed.

https://discuss.python.org/t/python-3-12-is-now-security-fix-only/87520

@hugovk hugovk removed the needs backport to 3.12 only security fixes label Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Assignees

@vstinner vstinner

Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @serhiy-storchaka @bedevere-bot @hugovk