Skip to content

improvement(tests+ci): phase 3 — shared-mock convergence completion and CI runner-minute cuts#5875

Merged
waleedlatif1 merged 8 commits into
stagingfrom
test/isolation-phase3
Jul 23, 2026
Merged

improvement(tests+ci): phase 3 — shared-mock convergence completion and CI runner-minute cuts#5875
waleedlatif1 merged 8 commits into
stagingfrom
test/isolation-phase3

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Shared-mock infrastructure (@sim/testing): complete stateful mocks — with set*/reset* APIs and 20 contract tests — for @/lib/core/config/env, @/lib/core/utils/urls (6 missing exports added), @/lib/core/config/redis, and @/lib/environment/utils, all installed globally in vitest.setup (same proven pattern as the db and env-flags mocks from #5856/#5871).

Redundant-mock dedup (~250 test files, net ~−800 lines): deleted file-local vi.mock factories that replicated the global mocks (95 @sim/db replicas in lib/ alone, 43 in app/) and converted customizing factories to configuring the shared mocks. ~40 files keep custom factories with documented reasons (bespoke assertion shapes, import-time env reads, tables re-exported from @sim/db).

CI runner-minute cuts (verified against 600-run history, no required-check risk — rulesets were read directly and contain none):

  • Fail-open dedup gate for promotion-PR test runs (39 duplicate runs measured in 5 days; ≈10,900 vcpu-min/mo). Every failure path runs the tests; worst case is status quo plus a small probe job
  • companion-pr-check concurrency (superseded runs raced the sticky-comment upsert)
  • Two trivial jobs right-sized 4vcpu→2vcpu

Measured flip-readiness (honest): 3× shuffled isolate: false runs still fail 51–59 files (unchanged vs baseline) — the remaining blockers are the justified custom factories and the upstream vitest mock-registry bug (vitest-dev/vitest#10290, our repro posted). This PR's value is manageability + bill + correctness, not yet the flip.

Type of Change

  • Improvement

Testing

Full apps/sim suite green (13732/13733; sole failure is the known local ripgrep dependency, green in CI). packages/testing 39/39 incl. new contract tests. tsc clean in both packages; biome clean. Dedup gate logic reviewed line-by-line (fail-open on every error path; jq tested against a real run payload); it cannot be exercised until a real promotion-PR event.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…s, companion-pr-check concurrency, right-size trivial jobs

- ci.yml: new dedup-promotion gate skips the pull_request test-build on
  staging/main-headed promotion PRs only when the merge tree provably equals
  the head tree (empty base delta over the merge base) AND the push-event run
  at the same sha passed its test jobs (polled). Fail-open on any error/
  timeout/failure, job-level skip only (skipped job reports Success); verified
  no required status checks are configured on main/staging rulesets.
  Measured 39 duplicate PR runs / 5.15 days (~227/mo) at ~7.1 min each on
  8vcpu (~57 vcpu-min), probe costs ~9 vcpu-min worst case on 2vcpu.
- companion-pr-check.yml: per-PR concurrency group with cancel-in-progress so
  superseded synchronize/edit runs stop; no paths filter (check depends on PR
  body + cross-repo state, not changed files).
- detect-version and check-docs-changes: 4vcpu -> 2vcpu Blacksmith runners
  (pure shell / depth-2 checkout + path filter only).
…edis-config, environment-utils

Shared mock infrastructure for vitest isolate:false convergence:
- packages/testing/src/mocks/env.mock.ts: stateful envMock (live env proxy, setEnv/resetEnvMock, process.env fallback)
- packages/testing/src/mocks/urls.mock.ts: complete urlsMock with real-behavior default impls + resetUrlsMock
- packages/testing/src/mocks/redis-config.mock.ts: adds getRedisConnectionDefaults + resetRedisConfigMock
- packages/testing/src/mocks/environment-utils.mock.ts: new environmentUtilsMock + fns + reset
- contract tests: env.mock.test.ts, urls.mock.test.ts, redis-config.mock.test.ts, environment-utils.mock.test.ts
- packages/testing/src/mocks/index.ts: barrel exports
- apps/sim/vitest.setup.ts: global installs for env, urls, redis, environment/utils
- real-module tests unmocked: lib/core/config/env.test.ts, lib/core/config/redis.test.ts, lib/core/utils/urls.test.ts, tools/index.test.ts (urls)
- stubEnv/process.env fallout migrated to setEnv: lib/webhooks/providers/{revenuecat,rootly,instantly}.test.ts, app/api/auth/oauth2/authorize/route.test.ts
…iders and misc dirs (shared-worker readiness)
@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jul 23, 2026 2:01am

Request Review

@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are CI orchestration and test harness refactors; production runtime code is largely untouched, with fail-open behavior if the dedup probe fails.

Overview
CI adds a fail-open dedup-promotion job that can skip duplicate test-build on promotion PRs when merge-tree matches head and a push run at the same SHA already passed tests; companion-pr-check gets per-PR concurrency with cancel-in-progress; detect-version and check-docs-changes move to 2vcpu runners.

Tests (~250 files) drop redundant local vi.mock for @sim/db, @/lib/auth, env/redis/urls, and drizzle-orm, wiring instead to global @sim/testing helpers (authMockFns, setEnv/resetEnvMock, dbChainMockFns, etc.) with afterAll cleanup where env is mutated.

Reviewed by Cursor Bugbot for commit 5547b91. Configure here.

Comment thread .github/workflows/ci.yml
Comment thread packages/testing/src/mocks/urls.mock.ts
…urls mock fallbacks

- the dedup gate re-verifies merge-tree equivalence against the LIVE base
  tip at decision time, closing the window where the base branch gains real
  commits during the poll (frozen BASE_SHA check alone was stale)
- the urls mock's getBaseUrl protocol prefix and getBaseDomain parse
  fallback now follow the shared isProd flag, mirroring the real module
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates shared test mocks and reduces duplicate CI work. The main changes are:

  • Adds stateful shared mocks for environment, URL, Redis, and environment utilities.
  • Replaces redundant local mocks across the Sim test suite.
  • Skips duplicate promotion-PR test runs only when matching push coverage is confirmed.
  • Adds workflow concurrency and uses smaller runners for lightweight jobs.

Confidence Score: 5/5

This looks safe to merge.

  • The Redis mock now preserves the secure connection behavior covered by the earlier report.
  • Completed push runs without matching test jobs cause the pull-request tests to run.
  • No blocking issues remain in the reviewed fixes.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds a fail-closed promotion-PR deduplication gate and reduces runner size for lightweight jobs.
.github/workflows/companion-pr-check.yml Adds per-PR concurrency so newer checks cancel superseded runs.
packages/testing/src/mocks/redis-config.mock.ts Expands the shared Redis mock with reset support and production-aligned TLS defaults.
apps/sim/vitest.setup.ts Registers the new shared mocks for the Sim test suite.

Reviews (3): Last reviewed commit: "fix(ci): keep polling while nested test ..." | Re-trigger Greptile

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 142d708. Configure here.

Comment thread packages/testing/src/mocks/redis-config.mock.ts Outdated
Comment thread .github/workflows/ci.yml Outdated
… redis defaults mock

- the dedup gate no longer infers coverage from overall run conclusion when
  no 'Test and Build /' jobs match — a renamed or skipped test job now runs
  the tests instead of skipping them
- the shared getRedisConnectionDefaults mock mirrors the real TLS resolution
  (rediss:// to a raw IP requires REDIS_TLS_SERVERNAME and yields
  tls.servername)
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread .github/workflows/ci.yml Outdated
An in-progress push run lists its reusable-workflow jobs only after the
caller starts; nojobs is now terminal (fail closed) only once the run has
completed without them.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 5547b91. Configure here.

@waleedlatif1
waleedlatif1 merged commit dda602a into staging Jul 23, 2026
21 checks passed
@waleedlatif1
waleedlatif1 deleted the test/isolation-phase3 branch July 23, 2026 03:36
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