Skip to content

test(minimap): guard destroy interval pause - #15532

Open
yzxcj797 wants to merge 1 commit into
Comfy-Org:mainfrom
yzxcj797:test/minimap-destroy-pause-15278
Open

test(minimap): guard destroy interval pause#15532
yzxcj797 wants to merge 1 commit into
Comfy-Org:mainfrom
yzxcj797:test/minimap-destroy-pause-15278

Conversation

@yzxcj797

Copy link
Copy Markdown

Summary

  • clear the interval-pause spy after minimap setup
  • assert that destroy() performs exactly one additional pause
  • cover the interval leak path independently of the initial shouldPoll pause

Closes #15278

Test plan

  • Mutated check: removing pauseChangeDetection() from destroy() makes the guarded test fail with 0 calls instead of 1
  • pnpm exec vitest run src/renderer/extensions/minimap/composables/useMinimap.test.ts
  • pnpm exec oxfmt --check src/renderer/extensions/minimap/composables/useMinimap.test.ts
  • pnpm exec oxlint src/renderer/extensions/minimap/composables/useMinimap.test.ts --type-aware
  • pnpm exec eslint src/renderer/extensions/minimap/composables/useMinimap.test.ts --cache
  • pnpm typecheck

@yzxcj797
yzxcj797 requested a review from a team August 21, 2026 05:33
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 21, 2026
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

🎨 Storybook: 🚧 Building...

🎭 Playwright: ⏳ Running...

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 49ec40cd-c867-41ea-9b7d-c659b893ee42

📥 Commits

Reviewing files that changed from the base of the PR and between bb47e34 and bfb8367.

📒 Files selected for processing (1)
  • src/renderer/extensions/minimap/composables/useMinimap.test.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The minimap destruction test now clears interval-pause calls made during initialization and verifies that destruction pauses the change-detection interval exactly once. The existing request-animation-frame pause assertion remains unchanged.

Changes

Minimap test validation

Layer / File(s) Summary
Verify destruction pauses the interval
src/renderer/extensions/minimap/composables/useMinimap.test.ts
The test clears initialization-time interval pause calls and asserts exactly one interval pause during destruction. It retains the rAF pause assertion.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to bfb83

This localized test-only change strengthens coverage for minimap cleanup behavior without changing production code or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: drjkl


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
End-To-End Regression Coverage For Fixes ❓ Inconclusive The review context provides the changed file and PR description, but it does not provide the PR title or commit subjects required to verify the bug-fix signal. Provide the PR title and commit subjects, then reassess the end-to-end regression coverage conditions.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the minimap destroy-path interval-pause test change.
Description check ✅ Passed The description explains the test change, linked issue, and validation steps, although it omits the template headings for Changes and Review Focus.
Linked Issues check ✅ Passed The changes directly fix issue #15278 by isolating the destroy-path pause and asserting exactly one call after setup.
Out of Scope Changes check ✅ Passed The pull request changes only the minimap test required to address issue #15278 and adds no unrelated code.
Website End-To-End Regression Coverage ✅ Passed The PR changes only src/renderer/extensions/minimap/composables/useMinimap.test.ts. It changes no files under apps/website/src/ or apps/website/public/.
Adr Compliance For Entity/Litegraph Changes ✅ Passed The only changed file is src/renderer/extensions/minimap/composables/useMinimap.test.ts, so the entity/litegraph ADR check does not apply.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(minimap): the destroy-path interval pause assertion is vacuous

1 participant