Skip to content

Fix most ui tests on emscripten target #131705

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Oct 14, 2024

To fix the linker errors, we need to set the output extension to .js instead of .wasm. Setting the output to a .wasm file puts Emscripten into standalone mode which is effectively a distinct target. We need to set the runner to be node as well.

This fixes most of the ui tests. I fixed 4 additional tests with simple problems:

  • intrinsics/intrinsic-alignment.rs -- Two #[cfg] macros match for Emscripten so we got duplicate definition
  • structs-enums/rec-align-u64.rs -- same problem
  • issues/issue-12699.rs -- hangs so I disabled it
  • process/process-sigpipe.rs -- Not expected to work on Emscripten so I disabled it

Resolves #131666.

There are 7 more failing tests. I'll try to investigate more and see if I can fix them or at least understand why they happen.

  • abi/numbers-arithmetic/return-float.rs (problem with wasm treatment of noncanonical floats?)

  • async-await/issue-60709.rs -- linker error related to memcpy. Possible Emscripten bug?

  • backtrace/dylib-dep.rs -- Says "Not supported"

  • backtrace/line-tables-only.rs -- Says "Not supported"

  • no_std/no-std-unwind-binary.rs -- compiler says error: lang item required, but not found: eh_catch_typeinfo

  • structs-enums/enum-rec/issue-17431-6.rs -- One of the two compiler errors is missing

  • test-attrs/test-passed.rs

    r?workingjubilee r?jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 14, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@workingjubilee
Copy link
Member

This fixes most of the ui tests. I fixed 4 additional tests with simple problems:

Nice!

There are 7 more failing tests. I'll try to investigate more and see if I can fix them or at least understand why they happen.

No requirement that you fix them in this PR. I expect the backtrace tests may get an ignore for emscripten. It sounds like the only one of particular interest is async-await/issue-60709.rs

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I left some suggestions. The other failing tests are fine (i.e. not needed to be addressed in this PR). I would encourage opening a tracking issue for the remaining failing ui tests on emscripten targets for tracking purposes so they aren't lost when this PR is merged.

@jieyouxu
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2024
@jieyouxu jieyouxu self-assigned this Oct 15, 2024
To fix the linker errors, we need to set the output extension to `.js` instead
of `.wasm`. Setting the output to a `.wasm` file puts Emscripten into standalone
mode which is effectively a distinct target. We need to set the runner to be
`node` as well.

This fixes most of the ui tests. I fixed a few more tests with simple problems:
- `intrinsics/intrinsic-alignment.rs` and `structs-enums/rec-align-u64.rs` --
Two `#[cfg]` macros match for Emscripten so we got a duplicate definition of
`mod m`.
- `issues/issue-12699.rs` -- Seems to hang so I disabled it
- `process/process-sigpipe.rs` -- Not expected to work on Emscripten so I
disabled it
@hoodmane hoodmane force-pushed the fix-emscripten-tests branch from 236df30 to 1d6643c Compare October 15, 2024 12:26
@hoodmane
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2024
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, this LGTM now. I'll approve once PR CI is green.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 15, 2024

📌 Commit 1d6643c has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2024
@hoodmane
Copy link
Contributor Author

Thanks for the review @jieyouxu!

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 15, 2024
…mpiler-errors

Rollup of 7 pull requests

Successful merges:

 - rust-lang#129794 (uefi: Implement getcwd and chdir)
 - rust-lang#130568 (Make some float methods unstable `const fn`)
 - rust-lang#131521 (rename RcBox to RcInner for consistency)
 - rust-lang#131701 (Don't report `on_unimplemented` message for negative traits)
 - rust-lang#131705 (Fix most ui tests on emscripten target)
 - rust-lang#131733 (Fix uninlined_format_args in stable_mir)
 - rust-lang#131734 (Update `arm64e-apple-tvos` maintainer)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fc1ad2e into rust-lang:master Oct 15, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 15, 2024
Rollup merge of rust-lang#131705 - hoodmane:fix-emscripten-tests, r=jieyouxu

Fix most ui tests on emscripten target

To fix the linker errors, we need to set the output extension to `.js` instead of `.wasm`. Setting the output to a `.wasm` file puts Emscripten into standalone mode which is effectively a distinct target. We need to set the runner to be `node` as well.

This fixes most of the ui tests. I fixed 4 additional tests with simple problems:

- `intrinsics/intrinsic-alignment.rs` -- Two `#[cfg]` macros match for Emscripten so we got duplicate definition
- `structs-enums/rec-align-u64.rs` -- same problem
- `issues/issue-12699.rs` -- hangs so I disabled it
- `process/process-sigpipe.rs` -- Not expected to work on Emscripten so I disabled it

Resolves rust-lang#131666.

There are 7 more failing tests. I'll try to investigate more and see if I can fix them or at least understand why they happen.

- abi/numbers-arithmetic/return-float.rs (problem with [wasm treatment of noncanonical floats](https://webassembly.github.io/spec/core/exec/numerics.html#nan-propagation)?)
- async-await/issue-60709.rs -- linker error related to memcpy. Possible Emscripten bug?
- backtrace/dylib-dep.rs -- Says "Not supported"
- backtrace/line-tables-only.rs -- Says "Not supported"
- no_std/no-std-unwind-binary.rs -- compiler says `error: lang item required, but not found: eh_catch_typeinfo`
- structs-enums/enum-rec/issue-17431-6.rs -- One of the two compiler errors is missing
- test-attrs/test-passed.rs

    r?workingjubilee r?jieyouxu
@hoodmane hoodmane deleted the fix-emscripten-tests branch October 16, 2024 07:29
@workingjubilee workingjubilee added the O-emscripten Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL! label Oct 21, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc O-emscripten Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL! S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Development

Successfully merging this pull request may close these issues.

emcc and wasm-ld can't link our ui tests for wasm32-unknown-emscripten?
6 participants