Skip to content
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

fix(wpt): run async tests and collect results #724

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

huancheng-trili
Copy link
Collaborator

Context

Closes JSTZ-256.
JSTZ-256

Description

Fixes the wpt runner in jstz_api so that it covers async test cases. Asynchronous tests, including those wrapped in promises, were not properly executed because eval only runs the synchronous tests and puts async ones in the task queue. The runtime task queue was never touched afterwards and thus the async tests were simply abandoned. With changes in this PR, async test cases can be executed as expected, and also because of this, we don't need the Null test status because at the end of the async task chain, test suites are completed and therefore there is always a meaningful test status returned.

Note that the task queue must be executed after all synchronous tests finish, i.e. after all eval calls are made. I haven't fully figured out why, but my guess is that the test harness API has some assumptions about the test flow for each test suite, e.g. after async tests are done, other things are ignored, such that some test cases are dropped if run_jobs is called right after an eval call.

Manually testing the PR

env UPDATE_EXPECT=1 cargo test --test wpt

and more tests show up in wptreport.json. These tests are already defined in the test suites but they were never executed.

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.87%. Comparing base (373ec79) to head (dadfa5c).
Report is 1 commits behind head on main.

Files with missing lines Coverage Δ
crates/jstz_wpt/src/report.rs 0.00% <ø> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 373ec79...dadfa5c. Read the comment docs.

@huancheng-trili huancheng-trili marked this pull request as ready for review December 31, 2024 15:33
crates/jstz_api/tests/wpt.rs Show resolved Hide resolved
crates/jstz_api/tests/wpt.rs Outdated Show resolved Hide resolved
@huancheng-trili
Copy link
Collaborator Author

@johnyob do you have any other comments? I added some notes in wpt.rs explaining the use of unwrap and Option.

@johnyob johnyob assigned huancheng-trili and unassigned johnyob Jan 7, 2025
@huancheng-trili huancheng-trili force-pushed the huanchengchang-jstz-256 branch from 12fca23 to dadfa5c Compare January 7, 2025 16:38
@huancheng-trili huancheng-trili merged commit 94cfb5c into main Jan 7, 2025
5 checks passed
@huancheng-trili huancheng-trili deleted the huanchengchang-jstz-256 branch January 7, 2025 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants