Skip to content

[ php-wasm] Add test-unit-jspi in CI workflow #2285

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

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

mho22
Copy link
Collaborator

@mho22 mho22 commented Jun 18, 2025

Motivation for the change, related issues

Based on this comment

This is a work in progress trying to correctly run the different php-wasm-node tests in a JSPI mode

Implementation details

TBD

Testing Instructions (or ideally a Blueprint)

TBD

@mho22
Copy link
Collaborator Author

mho22 commented Jun 18, 2025

Well, only two test-unit-jspi failed tests. I didn't expect that.

@adamziel
Copy link
Collaborator

PHP 8.1 > Startup sequence > Should have access to raw request data via the php://input stream 90ms
→ expected 'Dӷ\u0000Dӷ\u0000"bar"}' to deeply equal '{"foo": "bar"}'

this is a genuine failure

PHP 7.3 – process crash > Does not crash due to an unhandled Asyncify error 613ms
→ php.run should have thrown an error or caused an unhandled rejection

This can be disabled when we're running JSPI

@mho22
Copy link
Collaborator Author

mho22 commented Jun 19, 2025

On it.

@adamziel
Copy link
Collaborator

Is anything missing here?

@mho22
Copy link
Collaborator Author

mho22 commented Jun 20, 2025

I'm a bit skeptical since the pull request was resolved unusually smoothly, with only two minor issues.

But, I think everything is in place. 😄

@adamziel
Copy link
Collaborator

Haha sometimes things just work :) sort out the description and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants