Conversation
Signed-off-by: mrhapile <allinonegaming3456@gmail.com>
Signed-off-by: mrhapile <allinonegaming3456@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR expands Boa’s Rust-level unit tests for the WeakSet builtin to cover additional ECMAScript edge cases, improving regression protection beyond test262 coverage.
Changes:
- Adds a
WeakSetbuiltin test module and wires it intoweak_set/mod.rs. - Introduces unit tests for primitive rejection in
WeakSet.prototype.add, primitive-safehas/delete, and duplicate object additions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/engine/src/builtins/weak_set/tests.rs | Adds baseline + edge-case WeakSet behavior tests (add/has/delete, primitives, duplicates). |
| core/engine/src/builtins/weak_set/mod.rs | Includes the test module under #[cfg(test)]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| TestAction::assert_native_error( | ||
| "ws.add(null);", | ||
| JsNativeErrorKind::Type, | ||
| "WeakSet.add: expected target argument of type `object`, got target of type `object`", |
There was a problem hiding this comment.
The null case asserts an error message where the "got" type is object (because typeof null is "object"), which makes the message read like there was no type mismatch ("expected object … got object"). This makes the test misleading/brittle and also locks in a confusing engine error string. Consider either (a) updating the WeakSet.add TypeError message to special-case null (e.g., report null explicitly) and then adjusting this assertion accordingly, or (b) changing this test to assert only that a TypeError is thrown for null without coupling to the exact message text.
| TestAction::assert_native_error( | |
| "ws.add(null);", | |
| JsNativeErrorKind::Type, | |
| "WeakSet.add: expected target argument of type `object`, got target of type `object`", | |
| TestAction::assert( | |
| "try { ws.add(null); false } catch (e) { e instanceof TypeError }", |
Test262 conformance changes
Tested main commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4914 +/- ##
===========================================
+ Coverage 47.24% 57.78% +10.54%
===========================================
Files 476 556 +80
Lines 46892 60739 +13847
===========================================
+ Hits 22154 35101 +12947
- Misses 24738 25638 +900 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR expands the WeakSet unit test coverage.
The initial WeakSet tests introduced basic behavioral tests.
#4912
This PR adds additional ECMAScript edge case coverage.
The following behaviors are now tested:
These tests ensure correct handling of ECMAScript §24.4 WeakSet
invariants and improve robustness during future refactors.
All tests follow the same pattern used by other builtin tests.