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

Typo fix in other.test_exceptions_stack_trace_and_message_wasm #21656

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Mar 29, 2024

This typo was introduced in #21559. But I wonder, how is the test still passing in the CI? It fails locally for me, which lead me to discover this.

This typo was introduced in emscripten-core#21559. But I wonder, how is the test
still passing in the CI? It fails locally for me, which lead me to
discover this.
@aheejin aheejin marked this pull request as ready for review March 29, 2024 22:08
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

That is weird... it also fails for me, using node 18.19.1 locally. I also can't see why it passes on CI...

@aheejin aheejin requested a review from sbc100 March 29, 2024 22:57
@@ -8815,7 +8815,7 @@ def test_exceptions_stack_trace_and_message(self):
'at (src.wasm.)?foo',
'at (src.wasm.)?main']

if '-fwasm-excpeptions' in self.emcc_args:
if '-fwasm-exceptions' in self.emcc_args:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason is passed in CI is that we use the emsdk version of node there (v16).

Since version 16 of node doesn't support wasm exceptions the require_wasm_eh helper will already force v8 for all wasm exceptions tests when running on CI.

Its only users of node v17 and above that would be effected by this typo.

@aheejin aheejin merged commit f8375b9 into emscripten-core:main Mar 29, 2024
29 checks passed
@aheejin aheejin deleted the typo_fix branch March 29, 2024 23:24
impact-maker pushed a commit to impact-maker/emscripten that referenced this pull request Apr 11, 2024
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

3 participants