Skip to content

fix(node:test): preserve mock state across tracker resets - #7101

Merged
proggeramlug merged 3 commits into
mainfrom
fix/6767-node-test-mock-reset
Aug 9, 2026
Merged

fix(node:test): preserve mock state across tracker resets#7101
proggeramlug merged 3 commits into
mainfrom
fix/6767-node-test-mock-reset

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Addresses two mock-function failures tracked in #6767:

  • node-suite/test/mock-fn/reset-multiple
  • node-suite/test/mock-fn/tracker-reset-restore

Perry now keeps call history when mocks are restored, retains per-context implementation state where Node does, and disassociates existing mocks when the tracker is reset so later restoreAll() calls do not affect them. Tracker reset now also clears mock timers.

Validation:

  • cargo check -p perry-runtime
  • ./run_parity_tests.sh --suite node-suite --module test --filter reset (3/3, including mock-timers/reset-cancels)
  • PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --suite node-suite --module test --filter restore (3/3)

Summary by CodeRabbit

  • Bug Fixes

    • Improved node:test mock reset and restore behavior.
    • Preserved mock call history and implementations where appropriate.
    • Ensured reset and restore-all operations affect only tracked mocks.
    • Fixed mock disassociation after resets.
    • Added Node.js-compatible mock timer resetting.
  • Documentation

    • Added changelog coverage for the updated mock behavior.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Mock tracking distinguishes tracked mocks from other states. Reset and restore-all use tracking, clear tracking flags, and reset mock timers. The changelog and version metadata record the updated behavior.

Changes

Mock restoration lifecycle

Layer / File(s) Summary
Track created mocks
crates/perry-runtime/src/node_submodules/test.rs, crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs
MockState records tracking status. Function-created mocks and test mock states are marked tracked.
Apply tracked restoration
crates/perry-runtime/src/node_submodules/test.rs, changelog.d/7101-node-test-mock-reset.md, CLAUDE.md, Cargo.toml
Reset restores tracked mocks, clears tracking flags, and resets mock timers. Restore-all uses the same tracked-mock filtering. The changelog and version metadata record the update.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • PerryTS/perry#7098: Both changes modify mock-state restoration and call or implementation reset semantics.
  • PerryTS/perry#7100: Both changes modify mock tracking and reset behavior in node_submodules/test.rs.
  • PerryTS/perry#7102: Both changes modify node:test mock restoration and tracking behavior.

Suggested labels: bug, parity

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: preserving mock state across node:test tracker resets.
Description check ✅ Passed The description explains the fixes, references issue #6767, lists concrete behavior changes, and provides validation commands and results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/6767-node-test-mock-reset

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-runtime/src/node_submodules/test.rs`:
- Around line 987-1008: Update the tracker reset flow around
restore_tracked_mocks and MOCK_STATES so each previously tracked mock has its
call history reset and state.once cleared before tracked is set to false. Keep
restore_tracked_mocks preservation-only, and ensure restoreAll continues
preserving these fields while mock.reset performs the cleanup.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dc37d83b-3c0d-476a-b1d2-fbbf6fa14971

📥 Commits

Reviewing files that changed from the base of the PR and between c86835f and 77764d4.

📒 Files selected for processing (2)
  • changelog.d/7101-node-test-mock-reset.md
  • crates/perry-runtime/src/node_submodules/test.rs

Comment thread crates/perry-runtime/src/node_submodules/test.rs
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@proggeramlug
proggeramlug force-pushed the fix/6767-node-test-mock-reset branch from 79ffe61 to 9e5f7a4 Compare August 9, 2026 14:36
@proggeramlug
proggeramlug force-pushed the fix/6767-node-test-mock-reset branch from 9e5f7a4 to 1b92c4e Compare August 9, 2026 14:57
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and merging as v0.5.1408.

Worth recording what the rebase found: about half of this PR had already landed independently. Main's prepare_mock_state_restore (#7098/#7100) already stopped clearing once/calls on restore, so the surviving delta is the tracked flag, disassociation on mock.reset(), and the mock-timer reset. Every conflicting hunk in create_restore_context resolved to main, which had deleted the function outright.

One resolution the author may want to revisit: the branch guarded the implementation reset with if matches!(state.restore, MockRestoreTarget::None). That guard was dropped — it is a no-op for both target fixtures (both use bare mock.fn), and main has since added prepare_mock_state_restore with a dedicated unit test asserting the unguarded behaviour. All 29 mock-fn fixtures were checked with it dropped.

Verification — two builds in separate target dirs (pristine origin/main vs the branch), --profile perry-dev with -p perry -p perry-runtime-static -p perry-stdlib-static, PERRY_RUNTIME_DIR pinned, archive mtimes confirmed to have moved:

run_parity_tests.sh --suite node-suite --module test, all 90 fixtures: main 48 pass / 42 fail → branch 50 / 40. The delta is exactly mock-fn/reset-multiple and mock-fn/tracker-reset-restore; 0 regressions across all 90.

cargo test -p perry-runtime --lib --no-fail-fast: 1945 passed, 0 failed. (An earlier run on this same tree reported 3 failures and main reported 1 — all timing-sensitive tests, on a box at load average 15.8 with three concurrent builds. The clean run is the one taken at face value; settling_many_keys_is_not_quadratic is a performance assertion and fails under load on main too.)

Lint: 19/19 workflow steps.

@proggeramlug
proggeramlug merged commit 10a107c into main Aug 9, 2026
0 of 13 checks passed
@proggeramlug
proggeramlug deleted the fix/6767-node-test-mock-reset branch August 9, 2026 15:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Cargo.toml`:
- Line 318: Restore the release metadata to the pre-release version: set
[workspace.package].version in Cargo.toml to 0.5.1407 and set CLAUDE.md’s
**Current Version:** value to 0.5.1407.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 10b46db9-2172-4498-af8a-314fdffd75f0

📥 Commits

Reviewing files that changed from the base of the PR and between 79ffe61 and 1b92c4e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • CLAUDE.md
  • Cargo.toml

Comment thread Cargo.toml

[workspace.package]
version = "0.5.1407"
version = "0.5.1408"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | sed -n '1,120p'

echo
echo "== Cargo.toml relevant lines =="
if [ -f Cargo.toml ]; then
  nl -ba Cargo.toml | sed -n '1,45p;300,322p'
  echo
  echo "workspace.package version context:"
  rg -n '\[workspace\.package\]|version\s*=' Cargo.toml
else
  echo "Cargo.toml missing"
fi

echo
echo "== CLAUDE.md relevant lines =="
if [ -f CLAUDE.md ]; then
  nl -ba CLAUDE.md | sed -n '1,20p'
else
  echo "CLAUDE.md missing"
fi

echo
echo "== current branch/commit context =="
git rev-parse --short HEAD
git status --short --untracked-files=no | sed -n '1,120p'

Repository: PerryTS/perry

Length of output: 5231


🏁 Script executed:

#!/bin/bash
set -euo pipefail

if command -v python3 >/dev/null 2>&1; then
  python3 - <<'PY'
from pathlib import Path
for target in ["Cargo.toml", "CLAUDE.md"]:
    p = Path(target)
    print(f"== {target} ==")
    if p.exists():
        lines = p.read_text().splitlines()
        start = (318 if target == "Cargo.toml" else 11)
        for i in range(max(1, start-2), min(len(lines), start+4)+1):
            print(f"{i:4}: {lines[i-1]}")
    else:
        print("missing")
PY
else
  echo "python3 not available"
fi

echo
echo "== current branch/commit context =="
git rev-parse --short HEAD
git status --short --untracked-files=no | sed -n '1,80p'

Repository: PerryTS/perry

Length of output: 689


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
Cargo = Path("CONTRIBUTING.md")
if Cargo.exists():
    lines = Cargo.read_text().splitlines()
    for i, line in enumerate(lines, 1):
        if "external contributor" in line.lower() or "maintainer" in line.lower() or "main" in line.lower() or "Cargo.toml" in line:
            print(f"{i}: {line}")
PY

echo
echo "== metadata version mentions =="
rg -n 'Current Version|0\.5\.140[0-9]|workspace\.package|version\s*=' Cargo.toml CLAUDE.md CONTRIBUTING.md | sed -n '1,80p' || true

echo
echo "== git diff summary =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat HEAD 2>/dev/null || true

echo
echo "== git diff Cargo/CLAUDE =="
git diff -- Cargo.toml CLAUDE.md 2>/dev/null | sed -n '1,120p' || true

Repository: PerryTS/perry

Length of output: 2810


Restore the maintainers-only release metadata.

This update changes both package and documented current version. For an external contributor PR, restore both values to the pre-release 0.5.1407; the maintainer updates [workspace.package].version and CLAUDE.md **Current Version:** during merge/release.

📍 Affects 2 files
  • Cargo.toml#L318-L318 (this comment)
  • CLAUDE.md#L11-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Cargo.toml` at line 318, Restore the release metadata to the pre-release
version: set [workspace.package].version in Cargo.toml to 0.5.1407 and set
CLAUDE.md’s **Current Version:** value to 0.5.1407.

Sources: Coding guidelines, Learnings

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.

1 participant