Skip to content

Commit dda602a

Browse files
waleedlatif1waleedlatif1
authored
improvement(tests+ci): phase 3 — shared-mock convergence completion and CI runner-minute cuts (#5875)
* chore(ci): cut redundant runner minutes — dedup promotion-PR test runs, 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). * improvement(testing): complete stateful shared mocks for env, urls, redis-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 * improvement(tests): drop redundant local mocks in executor/tools/providers and misc dirs (shared-worker readiness) * improvement(tests): drop redundant local mocks in app routes (shared-worker readiness) * improvement(tests): drop redundant local mocks in lib (shared-worker readiness) * fix(ci+testing): live base-tip recheck before dedup skip; prod-aware 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 * fix(ci+testing): fail-closed nojobs fallback in dedup gate; TLS-aware 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) * fix(ci): keep polling while nested test jobs have not appeared yet 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.
1 parent 9d8e14c commit dda602a

269 files changed

Lines changed: 1827 additions & 1794 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/ci.yml‎

Lines changed: 133 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,144 @@ permissions:
3434
contents: read
3535

3636
jobs:
37+
# Promotion PRs (staging→main etc.) double-run the test suite: every commit
38+
# on staging/main already gets a push-event run of the exact same test-build,
39+
# and the pull_request "synchronize" run for the open release PR re-runs it on
40+
# the same sha seconds later (~39 duplicate runs / 5 days measured Jul 2026).
41+
# This gate skips the PR run's test-build ONLY when it can prove the identical
42+
# work already passed elsewhere:
43+
# 1. the PR base adds no file changes over the merge base with the head sha
44+
# (compare head...base has an empty diff), so the merge result's tree is
45+
# identical to the head tree the push run tested. Plain ancestry is not
46+
# enough of a check here: main's merge-only ruleset leaves merge commits
47+
# on main that staging lacks, so main...staging is permanently
48+
# "diverged" — but those merge commits carry no tree delta. A real
49+
# hotfix landed directly on the base makes the diff non-empty and we
50+
# run tests here;
51+
# 2. the push-event CI run at the same head sha finished its test jobs with
52+
# conclusion success (polled, since push + PR runs start simultaneously).
53+
# Fail-open by construction: any API error, timeout, missing run, or push-run
54+
# failure leaves covered=false and the PR run tests normally, so the PR check
55+
# is green only if tests passed either here or on the identical tree. Not a
56+
# workflow-level filter on purpose — a job-level skip still reports a
57+
# (successful) check context. NOTE: when test-build is skipped, its nested
58+
# "Test and Build / ..." contexts are not created; verified 2026-07-22 that
59+
# no required status checks are configured on main/staging (rulesets contain
60+
# only pull_request/deletion/non_fast_forward). If required checks are ever
61+
# added, require the caller "Test and Build" context, not the nested ones.
62+
# dev is excluded: push runs on dev skip test-build, so dev-headed PRs have
63+
# no push-run coverage to reuse.
64+
dedup-promotion:
65+
name: Dedup Promotion PR
66+
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
67+
timeout-minutes: 15
68+
if: >-
69+
github.event_name == 'pull_request' &&
70+
github.event.pull_request.head.repo.full_name == github.repository &&
71+
contains(fromJSON('["main", "staging"]'), github.event.pull_request.head.ref)
72+
permissions:
73+
contents: read
74+
actions: read
75+
outputs:
76+
covered: ${{ steps.probe.outputs.covered }}
77+
steps:
78+
- name: Probe for a passing push run at the same sha
79+
id: probe
80+
env:
81+
GH_TOKEN: ${{ github.token }}
82+
REPO: ${{ github.repository }}
83+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
84+
BASE_SHA: ${{ github.event.pull_request.base.sha }}
85+
BASE_REF: ${{ github.event.pull_request.base.ref }}
86+
run: |
87+
COVERED=false
88+
89+
# (1) Merge-tree equivalence: compare head...base diffs the merge
90+
# base against the base tip. An empty diff means the base contributes
91+
# nothing beyond what head already contains (merge commits only), so
92+
# the PR merge tree equals the head tree the push run tested. Any
93+
# error yields "unknown" and we run the tests.
94+
BASE_DELTA="$(gh api "repos/${REPO}/compare/${HEAD_SHA}...${BASE_SHA}" --jq '.files | length' 2>/dev/null || echo unknown)"
95+
if [ "$BASE_DELTA" != "0" ]; then
96+
echo "Base tip changes ${BASE_DELTA} file(s) over the merge base; merge tree differs from head — running tests in this PR run."
97+
echo "covered=false" >> "$GITHUB_OUTPUT"
98+
exit 0
99+
fi
100+
101+
# (2) Poll the push-event CI run's test jobs (they start seconds after
102+
# this run and take ~4 min). Any conclusion other than success, or
103+
# deadline expiry, falls through to covered=false.
104+
DEADLINE=$((SECONDS + 600))
105+
while [ "$SECONDS" -lt "$DEADLINE" ]; do
106+
RUN_JSON="$(gh api "repos/${REPO}/actions/workflows/ci.yml/runs?event=push&head_sha=${HEAD_SHA}&per_page=5" 2>/dev/null || echo '')"
107+
RUN_ID="$(printf '%s' "$RUN_JSON" | jq -r '.workflow_runs[0].id // empty' 2>/dev/null || echo '')"
108+
if [ -n "$RUN_ID" ]; then
109+
# Jobs from the reusable test-build workflow are prefixed
110+
# "Test and Build /". If that name ever changes, fall back to the
111+
# overall run conclusion (stricter, still correct).
112+
STATE="$(gh api "repos/${REPO}/actions/runs/${RUN_ID}/jobs?per_page=100" --jq '
113+
[.jobs[] | select(.name | startswith("Test and Build /"))] as $t |
114+
if ($t | length) == 0 then "nojobs"
115+
elif all($t[]; .conclusion == "success") then "success"
116+
elif any($t[]; .conclusion != null and .conclusion != "success") then "failed"
117+
else "pending" end' 2>/dev/null || echo pending)"
118+
if [ "$STATE" = "nojobs" ]; then
119+
# Nested reusable-workflow jobs appear only after the caller
120+
# starts, so an in-progress run with no "Test and Build /"
121+
# jobs yet just needs another poll. Once the run has
122+
# COMPLETED without them, fail closed: the job was renamed or
123+
# tests were skipped — never infer coverage from the overall
124+
# run conclusion. Update the prefix here on a rename.
125+
RUN_STATUS="$(printf '%s' "$RUN_JSON" | jq -r '.workflow_runs[0].status // "unknown"' 2>/dev/null || echo unknown)"
126+
if [ "$RUN_STATUS" = "completed" ]; then
127+
echo "Push run ${RUN_ID} completed with no 'Test and Build /' jobs — running tests in this PR run (update the prefix if the job was renamed)."
128+
break
129+
fi
130+
STATE="pending"
131+
fi
132+
if [ "$STATE" = "success" ]; then
133+
# (3) Re-verify merge-tree equivalence against the LIVE base
134+
# tip at decision time: the base branch may have gained real
135+
# commits during the poll, in which case the frozen BASE_SHA
136+
# check from step (1) is stale and the skip would be unsound.
137+
# Any error yields "unknown" and we run the tests.
138+
LIVE_DELTA="$(gh api "repos/${REPO}/compare/${HEAD_SHA}...heads/${BASE_REF}" --jq '.files | length' 2>/dev/null || echo unknown)"
139+
if [ "$LIVE_DELTA" = "0" ]; then
140+
COVERED=true
141+
echo "Push run ${RUN_ID} passed its test jobs for ${HEAD_SHA} and the live base tip still adds no file changes — skipping duplicate test-build."
142+
else
143+
echo "Base branch moved during the poll (live delta: ${LIVE_DELTA}) — running tests in this PR run."
144+
fi
145+
break
146+
elif [ "$STATE" = "failed" ]; then
147+
echo "Push run ${RUN_ID} did not pass (state: ${STATE}) — running tests in this PR run."
148+
break
149+
fi
150+
fi
151+
sleep 20
152+
done
153+
154+
echo "covered=${COVERED}" >> "$GITHUB_OUTPUT"
155+
37156
test-build:
38157
name: Test and Build
39-
if: github.ref != 'refs/heads/dev' || github.event_name == 'pull_request'
158+
needs: [dedup-promotion]
159+
# !cancelled(): dedup-promotion is skipped on every non-promotion event and
160+
# a skipped need would otherwise skip this job too. covered != 'true' is
161+
# fail-open — empty (skipped/failed probe) means run the tests.
162+
if: >-
163+
!cancelled() &&
164+
(github.ref != 'refs/heads/dev' || github.event_name == 'pull_request') &&
165+
needs.dedup-promotion.outputs.covered != 'true'
40166
uses: ./.github/workflows/test-build.yml
41167
secrets: inherit
42168

43169
# Detect if this is a version release commit (e.g., "v0.5.24: ...")
170+
# Smallest runner on purpose: a few seconds of pure shell over the commit
171+
# message, no checkout and no install.
44172
detect-version:
45173
name: Detect Version
46-
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }}
174+
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
47175
timeout-minutes: 5
48176
if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/dev')
49177
outputs:
@@ -486,9 +614,11 @@ jobs:
486614
fi
487615
488616
# Check if docs changed
617+
# Smallest runner on purpose: a depth-2 checkout plus a path filter, no
618+
# install and no build.
489619
check-docs-changes:
490620
name: Check Docs Changes
491-
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }}
621+
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
492622
timeout-minutes: 5
493623
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
494624
outputs:

‎.github/workflows/companion-pr-check.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ on:
2222
branches: [staging, main]
2323
workflow_dispatch: {}
2424

25+
# One live run per PR: a newer opened/edited/synchronize event supersedes the
26+
# previous run's work entirely (the sticky comment/label upsert is idempotent
27+
# and only the latest body matters), so cancel in-flight runs instead of
28+
# letting them race the new one. workflow_dispatch bulk scans get a unique
29+
# group via run_id and are never cancelled. No paths filter on purpose: the
30+
# check reads the PR body and cross-repo PR state, not changed files, so a
31+
# paths filter would both be semantically wrong and stop status refreshes.
32+
concurrency:
33+
group: companion-pr-check-${{ github.event.pull_request.number || github.run_id }}
34+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
35+
2536
permissions:
2637
pull-requests: write
2738
issues: write

‎apps/sim/app/account/settings/billing/credit-usage/page.test.ts‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,21 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { authMockFns } from '@sim/testing'
45
import { beforeEach, describe, expect, it, vi } from 'vitest'
56

6-
const {
7-
mockCreditUsageLoading,
8-
mockGetPersonalSubscription,
9-
mockGetSession,
10-
mockIsEnterprise,
11-
mockRedirect,
12-
} = vi.hoisted(() => ({
13-
mockCreditUsageLoading: vi.fn(() => null),
14-
mockGetPersonalSubscription: vi.fn(),
15-
mockGetSession: vi.fn(),
16-
mockIsEnterprise: vi.fn(),
17-
mockRedirect: vi.fn(),
18-
}))
7+
const { mockCreditUsageLoading, mockGetPersonalSubscription, mockIsEnterprise, mockRedirect } =
8+
vi.hoisted(() => ({
9+
mockCreditUsageLoading: vi.fn(() => null),
10+
mockGetPersonalSubscription: vi.fn(),
11+
mockIsEnterprise: vi.fn(),
12+
mockRedirect: vi.fn(),
13+
}))
1914

2015
vi.mock('next/navigation', () => ({
2116
redirect: mockRedirect,
2217
}))
2318

24-
vi.mock('@/lib/auth', () => ({
25-
getSession: mockGetSession,
26-
}))
27-
2819
vi.mock('@/lib/billing/core/plan', () => ({
2920
getHighestPriorityPersonalSubscription: mockGetPersonalSubscription,
3021
}))
@@ -44,6 +35,8 @@ vi.mock('@/app/workspace/[workspaceId]/settings/billing/credit-usage/loading', (
4435

4536
import AccountCreditUsagePage from '@/app/account/settings/billing/credit-usage/page'
4637

38+
const mockGetSession = authMockFns.mockGetSession
39+
4740
describe('AccountCreditUsagePage', () => {
4841
beforeEach(() => {
4942
vi.clearAllMocks()

‎apps/sim/app/api/audit-logs/export/route.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,23 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { createMockRequest } from '@sim/testing'
4+
import { authMockFns, createMockRequest } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

77
const {
8-
mockGetSession,
98
mockValidateEnterpriseAuditAccess,
109
mockBuildOrgScopeCondition,
1110
mockGetOrgWorkspaceIds,
1211
mockQueryAuditLogs,
1312
mockBuildFilterConditions,
1413
} = vi.hoisted(() => ({
15-
mockGetSession: vi.fn(),
1614
mockValidateEnterpriseAuditAccess: vi.fn(),
1715
mockBuildOrgScopeCondition: vi.fn(),
1816
mockGetOrgWorkspaceIds: vi.fn(),
1917
mockQueryAuditLogs: vi.fn(),
2018
mockBuildFilterConditions: vi.fn(),
2119
}))
2220

23-
vi.mock('@/lib/auth', () => ({
24-
auth: { api: { getSession: vi.fn() } },
25-
getSession: mockGetSession,
26-
}))
27-
2821
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
2922
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
3023
}))
@@ -38,6 +31,8 @@ vi.mock('@/app/api/v1/audit-logs/query', () => ({
3831

3932
import { GET } from '@/app/api/audit-logs/export/route'
4033

34+
const mockGetSession = authMockFns.mockGetSession
35+
4136
const ORG_ID = 'org-1'
4237
const MEMBER_IDS = ['admin-1']
4338
const SCOPE_SENTINEL = { type: 'org-scope-sentinel' }

‎apps/sim/app/api/auth/forget-password/route.test.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
*
44
* @vitest-environment node
55
*/
6-
import { createMockRequest } from '@sim/testing'
7-
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { createMockRequest, resetEnvMock, setEnv } from '@sim/testing'
7+
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88

99
const { mockRequestPasswordReset, mockLogger } = vi.hoisted(() => {
1010
const logger = {
@@ -22,9 +22,6 @@ const { mockRequestPasswordReset, mockLogger } = vi.hoisted(() => {
2222
}
2323
})
2424

25-
vi.mock('@/lib/core/utils/urls', () => ({
26-
getBaseUrl: vi.fn(() => 'https://app.example.com'),
27-
}))
2825
vi.mock('@/lib/auth', () => ({
2926
auth: {
3027
api: {
@@ -43,9 +40,14 @@ import { POST } from '@/app/api/auth/forget-password/route'
4340
describe('Forget Password API Route', () => {
4441
beforeEach(() => {
4542
vi.clearAllMocks()
43+
setEnv({ NEXT_PUBLIC_APP_URL: 'https://app.example.com' })
4644
mockRequestPasswordReset.mockResolvedValue(undefined)
4745
})
4846

47+
afterAll(() => {
48+
resetEnvMock()
49+
})
50+
4951
afterEach(() => {
5052
vi.clearAllMocks()
5153
})

‎apps/sim/app/api/auth/oauth/connections/route.test.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ vi.mock('@sim/db', () => ({
2525
eq: mockEq,
2626
}))
2727

28-
vi.mock('drizzle-orm', () => ({
29-
eq: mockEq,
30-
}))
31-
3228
vi.mock('jose', () => ({
3329
decodeJwt: mockDecodeJwt,
3430
}))

‎apps/sim/app/api/auth/oauth/disconnect/route.test.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,11 @@ import {
77
auditMock,
88
authMockFns,
99
createMockRequest,
10-
dbChainMock,
1110
dbChainMockFns,
1211
resetDbChainMock,
1312
} from '@sim/testing'
1413
import { beforeEach, describe, expect, it, vi } from 'vitest'
1514

16-
vi.mock('@sim/db', () => dbChainMock)
17-
1815
vi.mock('@sim/audit', () => auditMock)
1916

2017
import { POST } from '@/app/api/auth/oauth/disconnect/route'

‎apps/sim/app/api/auth/oauth/utils.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@
44
* @vitest-environment node
55
*/
66

7-
import { redisConfigMock, redisConfigMockFns } from '@sim/testing'
7+
import { redisConfigMockFns } from '@sim/testing'
88
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
99

1010
vi.mock('@/lib/oauth/oauth', () => ({
1111
refreshOAuthToken: vi.fn(),
1212
OAUTH_PROVIDERS: {},
1313
}))
1414

15-
vi.mock('@/lib/core/config/redis', () => redisConfigMock)
16-
1715
const { mockDecryptSecret } = vi.hoisted(() => ({ mockDecryptSecret: vi.fn() }))
1816
vi.mock('@/lib/core/security/encryption', () => ({
1917
decryptSecret: mockDecryptSecret,

‎apps/sim/app/api/auth/oauth2/authorize/route.test.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { createMockRequest, dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import {
5+
createMockRequest,
6+
dbChainMockFns,
7+
resetDbChainMock,
8+
resetEnvMock,
9+
setEnv,
10+
} from '@sim/testing'
11+
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
612

713
const {
814
mockGetSession,
@@ -16,8 +22,6 @@ const {
1622
mockGetCredentialActorContext: vi.fn(),
1723
}))
1824

19-
vi.mock('@sim/db', () => dbChainMock)
20-
2125
vi.mock('@/lib/auth/auth', () => ({
2226
auth: { api: { oAuth2LinkAccount: mockOAuth2LinkAccount } },
2327
getSession: mockGetSession,
@@ -70,10 +74,14 @@ function oauthCredentialActor(overrides: Record<string, unknown> = {}) {
7074
}
7175

7276
describe('OAuth2 authorize route', () => {
77+
afterAll(() => {
78+
resetEnvMock()
79+
})
80+
7381
beforeEach(() => {
7482
vi.clearAllMocks()
7583
resetDbChainMock()
76-
process.env.NEXT_PUBLIC_APP_URL = BASE_URL
84+
setEnv({ NEXT_PUBLIC_APP_URL: BASE_URL })
7785
mockGetSession.mockResolvedValue({ user: { id: USER_ID } })
7886
mockCheckWorkspaceAccess.mockResolvedValue({
7987
hasAccess: true,

0 commit comments

Comments
 (0)