fix(node:test): preserve mock state across tracker resets - #7101
Conversation
📝 WalkthroughWalkthroughMock 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. ChangesMock restoration lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
changelog.d/7101-node-test-mock-reset.mdcrates/perry-runtime/src/node_submodules/test.rs
77764d4 to
9b85911
Compare
9b85911 to
79ffe61
Compare
|
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. |
79ffe61 to
9e5f7a4
Compare
9e5f7a4 to
1b92c4e
Compare
|
Rebased onto current Worth recording what the rebase found: about half of this PR had already landed independently. Main's One resolution the author may want to revisit: the branch guarded the implementation reset with Verification — two builds in separate target dirs (pristine
Lint: 19/19 workflow steps. |
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
CLAUDE.mdCargo.toml
|
|
||
| [workspace.package] | ||
| version = "0.5.1407" | ||
| version = "0.5.1408" |
There was a problem hiding this comment.
🗄️ 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' || trueRepository: 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
Addresses two mock-function failures tracked in #6767:
node-suite/test/mock-fn/reset-multiplenode-suite/test/mock-fn/tracker-reset-restorePerry 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, includingmock-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
node:testmock reset and restore behavior.Documentation