-
-
Notifications
You must be signed in to change notification settings - Fork 446
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
Conversation
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.
Codecov ReportAttention: Patch coverage is
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. 🚀 New features to boost your workflow:
|
tests/wpt/src/lib.rs
Outdated
/// 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); | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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!
You can run the tests from the directory itself. The build script should download the WPT repository and any necessary files by itself.
Example:
The current set of curated tests are all working. Some tests have been marked as ignore with a TODO for further work.