Skip to content

fix(ci): count both sides of a rename in the desktop live gate, keep canary reports - #8751

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/desktop-ci-findings
Oct 7, 2026
Merged

waleedlatif1 merged 1 commit into
stagingfrom
fix/desktop-ci-findings

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Two CI review findings from #8748.

  • desktop-live-changes.sh: git diff --name-only lists only a rename's destination, so moving a live file into a skipped path (apps/docs/, .md, ...) skipped the live desktop suite. It now diffs with --no-renames, so both the deleted source and the added destination count.
  • desktop-e2e.yml: the electron@latest canary's Playwright step now sets the same BACKGROUND_EXECUTOR_REPORT_PATH / TERMINAL_CANCEL_REPORT_PATH as the pinned job, so its uploaded test-results include the JSON diagnostics.

Type of Change

  • Bug fix (CI)

Testing

  • New scripts/desktop-live-changes.test.ts runs the script against a temp clone with a local bare origin: rename apps/sim/x.ts -> apps/docs/x.ts gives changed=true (fails with changed=false without --no-renames), docs-only gives changed=false, an unfetchable base gives changed=true.
  • bun run test:scripts: 33 files, 357 tests pass.
  • actionlint clean on desktop-e2e.yml.

Checklist

  • Regression test added
  • No behavior change outside CI

@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Oct 7, 2026 6:08pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adjusts desktop test detection and adds test environment variables.

This PR appears safe to merge; no actionable issues were found.

What we checked:

  • Renames still run desktop tests: --no-renames exposes the deleted source as well as the destination. Any path outside the skip list makes the script return changed=true.

Summary

This PR fixes two desktop CI gaps.

  • desktop-live-changes.sh counts both sides of a rename, so moving a live file into a skipped path still runs the suite.
  • The Electron canary sets both report paths inside its uploaded test-results folder.
  • New tests cover renames, docs-only changes, and an unavailable base commit.

No actionable issues found. Tests were not run during this review.

Reviews (1) · Last reviewed commit: "fix(ci): count both sides of a rename in..." · Reviewed by Greptile

@waleedlatif1
waleedlatif1 merged commit b74b9dd into staging Oct 7, 2026
38 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/desktop-ci-findings branch October 7, 2026 19:19

This branch was successfully deployed

1 active deployment
Preview — c6ff860f Deployed Oct 7, 2026 by vercel[bot]
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