Skip to content

utils/build-script: add--test-with-wasm-runtime option #83573

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

utils/build-script: add--test-with-wasm-runtime option #83573

wants to merge 3 commits into from

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Aug 6, 2025 •
edited
Loading

With this change, passing --test-with-wasm-runtime=nodejs/--test-with-wasm-runtime=wasmkit to build-script allows switching between WasmKit and Node.js for running Wasm tests locally. Existing presets keep using WasmKit, so CI behavior is not impacted. Potentially, availability of a JS runtime allows writing tests that exercise JS interop for custom concurrency executors.

Differences between the runtimes:

  • with WasmKit on M4 check-swift-wasi-wasm32-custom target completes in 115 seconds with stdlib/PrintFloat16.swift the slowest test (115 seconds):

    Slowest Tests:
--------------------------------------------------------------------------
114.80s: Swift(wasi-wasm32) :: stdlib/PrintFloat16.swift
[...]

Testing Time: 114.95s

Total Discovered Tests: 1642
  Excluded         :   2 (0.12%)
  Unsupported      : 953 (58.04%)
  Passed           : 683 (41.60%)
  Expectedly Failed:   4 (0.24%)

  • with Node.js on M4 there's ~5x speedup in check-swift-wasi-wasm32-custom target at a cost of one failing test (reason TBC):

    Slowest Tests:
--------------------------------------------------------------------------
11.57s: Swift(wasi-wasm32) :: stdlib/PrintFloat16.swift
[...]

Failed Tests (1):
  Swift(wasi-wasm32) :: stdlib/StringIndex_bincompat.swift

Testing Time: 26.13s

Total Discovered Tests: 1642
  Unsupported      : 953 (58.04%)
  Passed           : 684 (41.66%)
  Expectedly Failed:   4 (0.24%)
  Failed           :   1 (0.06%)

@MaxDesiatov MaxDesiatov added build-script Area → utils: The build script WebAssembly Platform: WebAssembly labels Aug 6, 2025
@MaxDesiatov MaxDesiatov changed the title utils/build-script: add--test-with-wasm-runtime for runtime selection utils/build-script: add--test-with-wasm-runtime option Aug 6, 2025
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov MaxDesiatov added test suite Area: test suite cmake python Flag: Python source code labels Aug 6, 2025
@kateinoigakukun
Copy link
Member

I'm not a big fan of adding more and more wasm specific options in build-script and test/CMakeLists.txt just for local development TBH as we have to propagate things from build-script to lit.cfg correctly.
How about making node-wasi-runner to have a compatible CLI interface as utils/wasm-run.py and inject it to lit.cfg via env var like below:

config.target_run = os.environ.get('SWIFT_TEST_WASM_RUNTIME', os.path.join(config.swift_utils, 'wasm-run.py'))
env SWIFT_TEST_WASM_RUNTIME=$PWD/utils/wasm/node-wasi-runner ./utils/build-script -t

This gives us the flexibility to experiment more runtimes while keeping the build stuff clean

@MaxDesiatov
Copy link
Contributor Author

IMO env vars are significantly worse than build-script options as they're stringly typed, not validated at the earliest build stage, and also can't be easily surfaced/documented like options in build-script --help. If a developer makes a typo in env var name or value, they'll discover the error only at the latest stage of the build when it gets to testing.

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test macos

kateinoigakukun
kateinoigakukun approved these changes Aug 7, 2025
View reviewed changes
Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

If you feel strongly about this direction, I'm OK with going ahead as is.

utils/swift_build_support/swift_build_support/products/wasmstdlib.py
Comment on lines +223 to +224
print("wasmstdlib testing: Wasm runtime not found")
sys.exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

indent

utils/swift_build_support/swift_build_support/products/wasmstdlib.py
if not os.path.exists(wasmkit_bin_path) or not self.should_test_executable():

wasm_runtime_bin_path = self.infer_wasm_runtime(host_target)
print(f"wasm_runtime_bin_path is {wasm_runtime_bin_path}")
Copy link
Member

Choose a reason for hiding this comment

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

debug code?

kateinoigakukun
kateinoigakukun reviewed Aug 7, 2025
View reviewed changes
utils/wasm-run.py
command.append(f"{key}={envs[key]}")
command.append("--")
if args.runtime == 'nodejs':
command = [os.path.join(os.path.dirname(__file__), 'wasm', 'node-wasi-runner')]
Copy link
Member

@kateinoigakukun kateinoigakukun Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You need to forward env vars from collect_wasm_env and command line args as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't Node.js WASI inherit env vars by default? And args are passed below in command.extend(args)

Copy link
Member

@kateinoigakukun kateinoigakukun Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

oops, I missed the command args.

We need to set env vars prefixed with WASM_RUN_CHILD_ from host to guest while stripping the prefix. You can find it in collect_wasm_env.

kateinoigakukun
kateinoigakukun reviewed Aug 7, 2025
View reviewed changes
utils/swift_build_support/swift_build_support/products/wasmstdlib.py
Comment on lines +232 to +235
elif self.args.test_with_wasm_runtime == 'nodejs':
result = shutil.which('node')
if result:
return result
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 to add it to PATH again? isn't it already in PATH?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kateinoigakukun kateinoigakukun kateinoigakukun approved these changes

@bnbarham bnbarham Awaiting requested review from bnbarham

@etcwilde etcwilde Awaiting requested review from etcwilde etcwilde is a code owner

@justice-adams-apple justice-adams-apple Awaiting requested review from justice-adams-apple justice-adams-apple is a code owner

@shahmishal shahmishal Awaiting requested review from shahmishal shahmishal is a code owner

Assignees
No one assigned
Labels
build-script Area → utils: The build script cmake python Flag: Python source code test suite Area: test suite WebAssembly Platform: WebAssembly
Projects
Swift for Wasm
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaxDesiatov @kateinoigakukun