Skip to content
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

Improve testing of mainScriptUrlOrBlob. NFC #21728

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 9, 2024

I noticed when working on #21701 then these tests were not renaming the .js output, which meant that if mainScriptUrlOrBlob was completely ignored the tests would still pass.

These changes rename the output .js file such that the default name will not longer work and the tests would fail if mainScriptUrlOrBlob were ignored.

@sbc100 sbc100 requested a review from kripken April 9, 2024 20:13
I noticed when working on emscripten-core#21701 then these tests were not renaming
the `.js` output, which meant that if `mainScriptUrlOrBlob` was
completely ignored the tests would still pass.

These changes rename the output `.js` file such that the default
name will not longer work and the tests would fail if
`mainScriptUrlOrBlob` were ignored.
@sbc100 sbc100 requested a review from dschuff April 9, 2024 21:34
@sbc100 sbc100 enabled auto-merge (squash) April 9, 2024 21:35
@sbc100 sbc100 merged commit 3081a26 into emscripten-core:main Apr 9, 2024
29 checks passed
@sbc100 sbc100 deleted the mainScriptUrlOrBlob branch April 9, 2024 22:39
impact-maker pushed a commit to impact-maker/emscripten that referenced this pull request Apr 11, 2024
I noticed when working on emscripten-core#21701 then these tests were not renaming
the `.js` output, which meant that if `mainScriptUrlOrBlob` was
completely ignored the tests would still pass.

These changes rename the output `.js` file such that the default
name will not longer work and the tests would fail if
`mainScriptUrlOrBlob` were ignored.
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.

None yet

2 participants