Skip to content

gh-118422: Fix run_fileexflags() test#118429

Merged
vstinner merged 3 commits into
python:mainfrom
vstinner:run
Apr 30, 2024
Merged

gh-118422: Fix run_fileexflags() test#118429
vstinner merged 3 commits into
python:mainfrom
vstinner:run

Conversation

@vstinner
Copy link
Copy Markdown
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
Copy Markdown
Member

serhiy-storchaka commented Apr 30, 2024

!buildbot wasi

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Apr 30, 2024

🤖 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
Copy Markdown
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
Copy Markdown
Member Author

vstinner commented Apr 30, 2024

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

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Apr 30, 2024

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
Copy Markdown
Member

serhiy-storchaka commented Apr 30, 2024

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
@vstinner vstinner deleted the run branch April 30, 2024 20:32
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app Bot commented Apr 30, 2024

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
Copy Markdown

miss-islington-app Bot commented Apr 30, 2024

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
Copy Markdown
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