Repository navigation
Add WPT html/webappapis/timers tests - #8136
Conversation
![clanky-the-clanker[bot]](https://avatars.githubusercontent.com/in/5306670?s=60&v=4){kind=small}
There was a problem hiding this comment.
Superseded by the review of
95cb074e.
This review, of d317fca8
✅ Approved — No issues found. The new WPT target and test config follow the existing patterns, and every disabled or omitted test has a comment explaining why.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
📋 Code Quality: ✅ LGTM
No issues found in either file.
src/wpt/BUILD.bazel: the newwpt_testtarget uses the label@wpt//:html/webappapis/timers@module, the same form as the other nested-directory targets (dom/abort,fetch/api,wasm/jsapi).src/wpt/html/webappapis/timers-test.ts: it setsdisabledTests: trueandomittedTests: truethe way the harness supports them, gives each of those entries a comment, and ends withsatisfies TestRunnerConfig.
📝 Documentation: ✅ LGTM
No documentation issues. The comments on the disabled and omitted entries describe how the runtime currently behaves.
Reviewed commit: d317fca8
Run the .any.js tests from html/webappapis/timers, which cover
setTimeout and setInterval: clearing timers from callbacks, using
clearTimeout and clearInterval interchangeably, missing, undefined
and negative delays, and ordering of zero-delay timers.
All 10 subtests in the 7 enabled files pass, including the two
setup({single_test: true}) files that check negative delays.
- evil-spec-example.any.js is omitted: it relies on string handlers,
which workerd does not support because it disallows eval.
- type-long-settimeout.any.js and type-long-setinterval.any.js are
disabled because they hang. workerd clamps the delay as a double
rather than converting it to a WebIDL long, so 2 ** 32 ms does not
wrap to 0 and done() is never called.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d317fca to
95cb074
Compare
There was a problem hiding this comment.
✅ Approved — No issues found. The new WPT target follows the pattern of the existing targets, and every disabled or omitted test has a comment saying why.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
📋 Code Quality: ✅ LGTM
No issues found. The new wpt_test target in src/wpt/BUILD.bazel follows the pattern of the targets around it. src/wpt/html/webappapis/timers-test.ts uses disabledTests and omittedTests the way the harness supports them, and each disabled or omitted entry has its comment.
📝 Documentation: ✅ LGTM
No documentation issues. The comments on the disabled and omitted entries describe how the runtime behaves today.
Reviewed commit: 95cb074e
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## automatic-update-wpt #8136 +/- ##
=======================================================
Coverage ? 68.31%
=======================================================
Files ? 726
Lines ? 189662
Branches ? 29672
=======================================================
Hits ? 129568
Misses ? 42719
Partials ? 17375 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds a WPT target for
html/webappapis/timers: all 10 subtests in 7 files pass.The two
type-long-*files are disabled because they hang (delays aren't converted to a WebIDLlong), andevil-spec-exampleis omitted (string handlers).