Skip to content

[esm-integration] Add a new core test mode and fix or disable all tests #24288

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
merged 1 commit into from
May 16, 2025

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 9, 2025

No description provided.

@sbc100 sbc100 changed the title [esm-integration] Add a new core test mode and either fix or disable all tests [WIP] [esm-integration] Add a new core test mode and either fix or disable all tests May 9, 2025
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 9, 2025
This change also adds a new core test mode for ESM integration and
uses this to run all the variants of test_fs_js_api.

As part of this I also updated the octal constants in test_fs_js_api.c
which are required to be of the new form in strict JS.  All code in ES
modules is implicitly in strict mode.

Split out from emscripten-core#24288

See emscripten-core#24060
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 9, 2025
This change also adds a new core test mode for ESM integration and
uses this to run all the variants of test_fs_js_api.

As part of this I also updated the octal constants in test_fs_js_api.c
which are required to be of the new form in strict JS.  All code in ES
modules is implicitly in strict mode.

Split out from emscripten-core#24288

See emscripten-core#24060
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 9, 2025
This change also adds a new core test mode for ESM integration and
uses this to run all the variants of test_fs_js_api.

As part of this I also updated the octal constants in test_fs_js_api.c
which are required to be of the new form in strict JS.  All code in ES
modules is implicitly in strict mode.

Split out from emscripten-core#24288

See emscripten-core#24060
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 9, 2025
This change also adds a new core test mode for ESM integration and
uses this to run all the variants of test_fs_js_api.

As part of this I also updated the octal constants in test_fs_js_api.c
which are required to be of the new form in strict JS.  All code in ES
modules is implicitly in strict mode.

Split out from emscripten-core#24288

See emscripten-core#24060
sbc100 added a commit that referenced this pull request May 9, 2025
This change also adds a new core test mode for ESM integration and uses
this to run all the variants of test_fs_js_api.

As part of this I also updated the octal constants in test_fs_js_api.c
which are required to be of the new form in strict JS. All code in ES
modules is implicitly in strict mode.

Split out from #24288

See #24060
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 13, 2025
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 13, 2025
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 13, 2025
sbc100 added a commit to sbc100/emscripten that referenced this pull request May 13, 2025
sbc100 added a commit that referenced this pull request May 13, 2025
These tests just required updating to handle `.mjs` rather than `.js`
output.

Split out from #24288

See #24060
@sbc100 sbc100 force-pushed the esm_integration branch 2 times, most recently from 36c8a41 to 693533d Compare May 16, 2025 20:45
@sbc100 sbc100 changed the title [WIP] [esm-integration] Add a new core test mode and either fix or disable all tests [esm-integration] Add a new core test mode and fix or disable all tests May 16, 2025
@sbc100 sbc100 force-pushed the esm_integration branch 2 times, most recently from 21c6eda to 874faab Compare May 16, 2025 20:49
@sbc100 sbc100 requested review from brendandahl and dschuff May 16, 2025 20:50
@sbc100 sbc100 force-pushed the esm_integration branch 9 times, most recently from bd8b23f to b39ce99 Compare May 16, 2025 21:35
@@ -1142,6 +1153,9 @@ workflows:
- test-modularize-instance:
requires:
- build-linux
- test-esm-integration:
Copy link
Member

Choose a reason for hiding this comment

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

do we need both the modularize-instance mode and the esm integration mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the time being I think it would be good. Maybe we can delete the former completely once this lands.

@@ -7796,6 +7824,7 @@ def test_embind_dylink_visibility_hidden(self):
self.do_runf('main.cpp', 'done\n', emcc_args=['--bind'])

@no_wasm2js('TODO: source maps in wasm2js')
@no_esm_integration('WASM_ESM_INTEGRATION is not compatible with dwarf output')
Copy link
Member

Choose a reason for hiding this comment

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

why is this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think its because of the wasm-dis / wasm-as round-trip we have to do to modify the import names. Hopefully that can be fixed soon.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah. SGTM to have this now and fix soon.

@sbc100 sbc100 force-pushed the esm_integration branch 2 times, most recently from 47d11b6 to 8779ae3 Compare May 16, 2025 22:07
…all test.

This is fairly larger change which can likely to split up before landing.
@sbc100 sbc100 force-pushed the esm_integration branch from 8779ae3 to 347e140 Compare May 16, 2025 22:52
@sbc100 sbc100 merged commit a5b003d into emscripten-core:main May 16, 2025
2 of 13 checks passed
@sbc100 sbc100 deleted the esm_integration branch May 16, 2025 22:52
@RReverser
Copy link
Collaborator

Looks like this broke CI because it can't find any tests matching the pattern.

sbc100 added a commit to sbc100/emscripten that referenced this pull request May 17, 2025
sbc100 added a commit that referenced this pull request May 17, 2025
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.

4 participants