Skip to content

test(vscode): quarantine flaky diff-panel visual checks - #14238

Closed
marius-kilocode wants to merge 2 commits into
mainfrom
test/quarantine-flaky-diff-panels
Closed

marius-kilocode wants to merge 2 commits into
mainfrom
test/quarantine-flaky-diff-panels

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

What changed

Quarantines two flaky diff-panel checks that were blocking unrelated PRs on Linux CI:

  • agentmanager--diff-panel-with-diffs is added to the visual-regression SKIP set. Its generated baseline changed twice within one workflow run, so the "fail if baselines still changing after auto-update" guard could not pass. The related agentmanager--diff-panel-scroll-up story is already quarantined there.
  • keeps the inline diff position stable when the row width changes in tests/diff-scroll-preservation.spec.ts is skipped in CI only. It failed on Linux with result.correction 5520 against the allowed <= 1, on two runs at different commits, while passing locally, including 4/4 with CI parallelism and under 6x CPU throttling. The test still runs locally.

Why

Both checks use the nondeterministic diff-panel-scroll-up inline diff story. The scroll-clamp behavior behind the failure still deserves a real fix from the inline-diff owners, but it cannot be reproduced or validated from macOS.

Verification

  • bunx playwright test tests/diff-scroll-preservation.spec.ts -g "inline diff position stable" passes locally (2 passed).
  • CI=true bunx playwright test tests/diff-scroll-preservation.spec.ts -g "keeps the inline diff position stable when the row width changes" reports 1 skipped.
  • bunx prettier --check passes on both changed spec files.

// the same assertion passes locally and in most CI runs. The shared
// diff-panel-scroll-up story is already quarantined in visual-regression.spec.ts
// for the same nondeterministic scroll offset.
test.skip(!!process.env["CI"], "Flaky Linux virtualization timing on the diff-panel-scroll-up story")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[SUGGESTION]: This turns off all CI coverage for the width-change scroll-clamp behavior

Gating on process.env["CI"] makes the check dark in every CI run, and per the PR description the failure is an intermittent real scroll clamp (a full panel height, not just a pixel-level render difference), so the only automated guard against that behavior stops running. That is a reasonable call given it cannot be reproduced from macOS, but there is no tracking reference or re-enable condition attached, so the defect this test documents can regress silently and the quarantine is easy to forget. Consider linking a tracking issue (for example a TODO(#issue) next to the skip) so it has an owner.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts 379 CI-gated test.skip disables the only automated guard for the scroll-clamp behavior with no tracking reference or re-enable condition.
Files Reviewed (3 files)
  • packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts - 1 issue
  • packages/kilo-vscode/tests/visual-regression.spec.ts - no issues
  • packages/kilo-vscode/tests/visual-regression.spec.mts - no issues

Notes

  • The added agentmanager--diff-panel-with-diffs SKIP entry matches the existing DiffPanelWithDiffs story id and follows the repo's documented quarantine mechanism; the story's baseline is not referenced from docs, so skipping it is safe.
  • test.skip(!!process.env["CI"], ...) is a supported Playwright in-body conditional skip and mirrors the process.env["CI"] gating already used in playwright.config.ts.
  • The kilocode-landing SEO/content rules do not apply to this PR; it touches no pages or markdown.
  • No memory-leak or security-relevant changes: the diff is test-only.

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@marius-kilocode

Copy link
Copy Markdown
Collaborator Author

Closing: the flaky diff-panel checks will be fixed at the root in #14226 instead of quarantined.

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