Skip to content

Add WPT as optional tests for boa_runtime #4008

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 37 commits into from
Jul 7, 2025
Merged

Add WPT as optional tests for boa_runtime #4008

merged 37 commits into from
Jul 7, 2025

Conversation

hansl
Copy link
Contributor

@hansl hansl commented Sep 26, 2024

You can run the tests from the directory itself. The build script should download the WPT repository and any necessary files by itself.

Example:

cd tests/wpt
cargo test

The current set of curated tests are all working. Some tests have been marked as ignore with a TODO for further work.

Some methods are NOT currently supported (some don' make sense
outside of a browser context). They are still implemented but
will throw a JavaScript Error.

Supported methods should follow the specification perfectly.
To run the tests, you require adding the `wpt-tests` feature
and setting the `WPT_ROOT` environment variable on the
command line. This is currently depending on a pending rstest
PR: la10736/rstest#277

The current set of curated tests are all working.
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 40.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 50.42%. Comparing base (6ddc2b4) to head (64ee6f8).
Report is 460 commits behind head on main.

Files with missing lines Patch % Lines
core/runtime/src/console/mod.rs 25.00% 6 Missing ⚠️
core/runtime/src/interval.rs 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4008      +/-   ##
==========================================
+ Coverage   47.24%   50.42%   +3.18%     
==========================================
  Files         476      498      +22     
  Lines       46892    49893    +3001     
==========================================
+ Hits        22154    25160    +3006     
+ Misses      24738    24733       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hansl hansl marked this pull request as ready for review November 5, 2024 04:54
@nekevss nekevss requested a review from a team November 5, 2024 15:44
@nekevss nekevss added the enhancement New feature or request label Nov 5, 2024
@hansl hansl requested a review from jedel1043 June 30, 2025 13:56
Comment on lines 377 to 395
/// Test the URL class with the WPT test suite.
// A bunch of these tests are failing due to lack of support in the URL class,
// or missing APIs such as fetch.
#[cfg(not(clippy))]
#[rstest::rstest]
fn url(
#[base_dir = "${WPT_ROOT}"]
#[files("url/url-*.any.js")]
#[exclude("idlharness")]
// "Base URL about:blank cannot be a base"
#[exclude("url-searchparams.any.js")]
// "fetch is not defined"
#[exclude("url-origin.any.js")]
#[exclude("url-setters.any.js")]
#[exclude("url-constructor.any.js")]
path: PathBuf,
) {
execute_test_file(&path);
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems fine for now since we only need to test a small subset of the suite, but do you plan to eventually migrate the tests to something like boa_tester?

I ask this because it just feels very boilerplatey having to manually declare every test we want to run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests could simply be #[files("*/*.js")], but there is a lot more WPT than test262 that we don't support (or ever plan to). So this makes sense. Also, unit tests make sense as it hooks into the Rust test framework and allows all reporting options (like coverage) that cargo test supports.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, if there's a way to run big sets of tests then that's fine.

unit tests make sense as it hooks into the Rust test framework and allows all reporting options (like coverage) that cargo test supports.

I'm up for integrating with cargo test, but let's just take note that hooking into the Rust test framework has its disadvantages, like the hacks that we need to do to make it possible to run cargo test without having to run the wpt suite. I think it makes sense in this case because the subset of wpt that we support is so small that we don't need advanced filtering/collection/categorization of tests, but this wouldn't work for test262.

@hansl hansl requested a review from jedel1043 July 1, 2025 14:02
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Thank you for addressing all my suggestions!

@jedel1043 jedel1043 added the Internal Category for changelog label Jul 7, 2025
@jedel1043 jedel1043 added this to the next-release milestone Jul 7, 2025
@hansl hansl added this pull request to the merge queue Jul 7, 2025
Merged via the queue into boa-dev:main with commit cc83c51 Jul 7, 2025
16 checks passed
@hansl hansl deleted the wpt branch July 7, 2025 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Internal Category for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants