Skip to content

Wasm unit tests on CI#1275

Merged
Bromeon merged 3 commits intogodot-rust:masterfrom
PgBiel:wasm-unit-tests
Feb 3, 2026
Merged

Wasm unit tests on CI#1275
Bromeon merged 3 commits intogodot-rust:masterfrom
PgBiel:wasm-unit-tests

Conversation

@PgBiel
Copy link
Copy Markdown
Contributor

@PgBiel PgBiel commented Aug 15, 2025 •
edited
Loading

Runs wasm unit tests on CI. Browsers / exports not tested here yet (left to a future PR).

Full details here https://typst.app/project/r314DoDea5XiYYr5bq5IFP

To summarize:

P.S. I'll be organizing commits later, sorry for the mess :)

TODO:

  • Add command to check.sh
  • Test nothreads
  • Ignore parallel tests in nothreads
  • Ignore plugin test (this should be unignored later by Use .init_array to call constructors in WASM instead of JS snippet #1476)
  • Fix godot4-prebuilt templates assuming 64-bit architecture (instead of wasm 32-bit) leading to failing struct size assertions
  • Fix test_global_would_block test in global.rs when testing Wasm nothreads For now, test ignored
  • Add to minimal-ci
  • Add to full-ci

@GodotRust
Copy link
Copy Markdown

GodotRust commented Aug 15, 2025

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1275

@PgBiel PgBiel force-pushed the wasm-unit-tests branch 3 times, most recently from 8f3e5bd to 556bb1c Compare August 15, 2025 02:08
@Bromeon Bromeon force-pushed the master branch 2 times, most recently from 779c043 to ddc5b76 Compare August 18, 2025 19:16
Bromeon
Bromeon reviewed Aug 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

I'll also rebase onto lastest master, to fix a CI issue that has been merged in the meantime.

check.sh Outdated
Comment on lines +196 to +197
# More memory (256 MiB) is needed for the parallel godot-cell tests which
# spawn 70 threads each.
Copy link
Copy Markdown
Member

@Bromeon Bromeon Aug 21, 2025

Choose a reason for hiding this comment

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

Good observation. Do you think this still makes sense, pushing the limits also on web?

Copy link
Copy Markdown
Contributor Author

@PgBiel PgBiel Aug 22, 2025

Choose a reason for hiding this comment

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

I think it's fine for what it's worth. We could consider reducing the tests a bit, but I'd rather get existing tests to work first.

PgBiel
PgBiel commented Aug 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@PgBiel PgBiel 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 taking a look. I've been busy with other tasks lately, so sorry for the slow progress, but don't worry, we will get this through the finish line :)

check.sh Outdated
Comment on lines +196 to +197
# More memory (256 MiB) is needed for the parallel godot-cell tests which
# spawn 70 threads each.
Copy link
Copy Markdown
Contributor Author

@PgBiel PgBiel Aug 22, 2025

Choose a reason for hiding this comment

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

I think it's fine for what it's worth. We could consider reducing the tests a bit, but I'd rather get existing tests to work first.

@Bromeon Bromeon added this to the 0.4.x milestone Sep 14, 2025
@Bromeon Bromeon added feature Adds functionality to the library c: tooling CI, automation, tools labels Sep 14, 2025
@Bromeon
Copy link
Copy Markdown
Member

Bromeon commented Sep 14, 2025

Postponed to after initial 0.4.0 release, as it should be a non-breaking addition.

@PgBiel PgBiel mentioned this pull request Oct 4, 2025
Closed
@Bromeon Bromeon modified the milestones: 0.4.x, 0.5 Dec 4, 2025
@ufoot ufoot mentioned this pull request Dec 15, 2025
Closed
@PgBiel PgBiel mentioned this pull request Jan 10, 2026
Merged
SvizelPritula
SvizelPritula reviewed Jan 10, 2026
View reviewed changes
@Bromeon
Copy link
Copy Markdown
Member

Bromeon commented Jan 11, 2026

Now that #1479 is merged, it's possible to build Wasm without api-custom.

You could maybe rebase this? 🙂

@PgBiel PgBiel force-pushed the wasm-unit-tests branch 2 times, most recently from 5dca132 to f84c675 Compare January 11, 2026 22:29
@PgBiel PgBiel marked this pull request as ready for review January 27, 2026 13:21
Bromeon
Bromeon reviewed Jan 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Added some comments, just superficial ones. A small nitpick, you have extremely short comment lines that spread over many lines, while convention is 120-145 characters per line 🙂 only fix if it's no effort, otherwise I'll do it later...

I think the PR could be squashed and should then be ready!

@PgBiel
Copy link
Copy Markdown
Contributor Author

PgBiel commented Jan 29, 2026

@Bromeon i've split the command into testwebThreads and testwebNothreads, in order to separate the two tests for more useful output (know which one failed). What do you think of this design (see the second commit)?

@Bromeon
Copy link
Copy Markdown
Member

Bromeon commented Feb 3, 2026

Yes, very nice! I tweaked check.sh slightly -- I also hid the atomics unstable warnings (apparently there's no by-design way to do that, so I used grep -v 😀).

Thanks a lot for your great work! 🚀

@Bromeon Bromeon added this pull request to the merge queue Feb 3, 2026
@Bromeon
Copy link
Copy Markdown
Member

Bromeon commented Feb 3, 2026

Btw, over time we may need to trim the CI a bit -- there are currently 33 jobs run in full CI (which is 13 more than GitHub's concurrency limit). Big part of that is compatibility checks with Godot 4.2-4.6, but also feature combinations, safeguard variants, miri, etc.

But for now Wasm coverage is very important, so it's good to check both threads + nothreads.

Merged via the queue into godot-rust:master with commit fca0a33 Feb 3, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bromeon Bromeon Bromeon left review comments

+1 more reviewer

@SvizelPritula SvizelPritula SvizelPritula left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: tooling CI, automation, tools feature Adds functionality to the library

Projects

None yet

Milestone

0.5

Development

Successfully merging this pull request may close these issues.

4 participants

@PgBiel @GodotRust @Bromeon @SvizelPritula