Skip to content

gh-127146: Skip test_readinto_non_blocking on Emscripten #129421

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

Merged

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jan 28, 2025

non_blocking doesn't really work on Emscripten.

non_blocking doesn't really work on Emscripten
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

👍

@freakboy3742 freakboy3742 merged commit 7893908 into python:main Jan 28, 2025
47 checks passed
@cmaloney
Copy link
Contributor

@hoodmane, @freakboy3742 rather than a platform-specific check for this one test, could you make it so the set_blocking / get_blocking general platform feature flag which the test is also guarded by fails instead? Are a number of other tests in test_os that use that sort of check. Worried this platform specific check would end up needing to be copied to general libraries to support Emscripten. Python's io library has strong non-blocking support in its lower layers, and a number of projects / libraries rely on specific guarantees + file bugs and fixes.

@freakboy3742
Copy link
Contributor

@cmaloney That's definitely an approach that we could take - it's an approach that iOS and Android use for a bunch of APIs in the "they exist, but you can't call them" category. However, in this case, it's not 100% clear if the issue here is a bug in Emscripten itself. If it is, then a test skip seems more appropriate. If it turns out there is a fundamental limitation that prevents Emscripten from avoiding the problem causing this test failure, then I'd agree guarding the entire feature would make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants