Skip to content

Commit c072de6

Browse files
Ralphclaude
andcommitted
test: #5841 — correct misleading doc comment on existing-var test
CodeRabbit review on PR #5855 suggested asserting that a pre-existing global var's `configurable: false` descriptor survives an eval-triggered value update. That assertion would fail today: Perry does not yet reify a top-level *script* `var` declaration as a real `globalThis` own-property until something touches it via reflection, so `hasOwnProperty` reads `false` before the eval runs in this test, and the create-if-absent prelude (correctly, per its own contract) creates a fresh `configurable: true` binding instead of finding a pre-existing one to leave alone. That's a separate, pre-existing gap in how Perry models module-top `var` (tracked by the still-failing test262 `var-env-var-init-global-exstng` case), orthogonal to this PR's eval-configurable-descriptor fix. Correct the test's doc comment instead of adding a would-be-failing assertion, so it no longer overclaims behavior Perry doesn't actually have yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 7aaf06f commit c072de6

1 file changed

Lines changed: 11 additions & 4 deletions

File tree

crates/perry/tests/issue_5841_eval_global_descriptor.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,17 @@ fn direct_eval_new_global_var_binding_is_configurable() {
107107
}
108108

109109
/// A `var` declared inside `eval` for a name that already exists as a global
110-
/// (declared by the enclosing script itself, non-configurable) must only
111-
/// update the value — the pre-existing descriptor's `configurable: false`
112-
/// survives untouched (test262 `language/eval-code/direct/var-env-var-init-
113-
/// global-exstng`).
110+
/// must update the value via a plain assignment, not the create-if-absent
111+
/// `Object.defineProperty` prelude this PR touches (test262 `language/eval-
112+
/// code/direct/var-env-var-init-global-exstng`, which additionally asserts the
113+
/// pre-existing descriptor's `configurable: false` survives untouched — not
114+
/// checked here: Perry does not yet reify a top-level *script* `var`
115+
/// declaration as a real `globalThis` own-property until something touches it
116+
/// via reflection, so `hasOwnProperty` reads `false` before the eval runs and
117+
/// the create-if-absent prelude (correctly, per its own contract) creates a
118+
/// fresh `configurable: true` binding — a separate, pre-existing gap in how
119+
/// Perry models module-top `var`, orthogonal to this PR's eval-configurable
120+
/// fix and out of scope here).
114121
const DIRECT_EXISTING_VAR_VALUE_UPDATED: &str = r#"
115122
var __perry_5841_existing = 23;
116123
eval("var __perry_5841_existing = 45;");

0 commit comments

Comments
 (0)