Skip to content

Commit b6a23cf

Browse files
roomote[bot]roomoteedelauna
authored
[Chore] Improve core coverage CI and merge queue readiness (#207)
* chore: split packages/core integration coverage lane * [Chore] Fix core integration lane follow-up issues * [Chore] Relax Windows worktree integration matching * [Chore] Align worktree delete integration assertion with service contract * chore: Move core dual-lane test:coverage inside the package * chore: Flag core unit/integration Codecov uploads separately * chore: tighten core coverage lane wiring * Optimize CI cache and coverage uploads * chore: add merge queue triggers for required CI --------- Co-authored-by: Roomote <roomote@roocode.com> Co-authored-by: Elliott de Launay <edelauna@gmail.com>
1 parent 79ed8b5 commit b6a23cf

14 files changed

Lines changed: 785 additions & 516 deletions

.github/workflows/code-qa.yml

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ on:
66
branches: [main]
77
pull_request:
88
types: [opened, reopened, ready_for_review, synchronize]
9+
merge_group:
10+
types: [checks_requested]
911

1012
jobs:
1113
check-translations:
@@ -49,33 +51,73 @@ jobs:
4951
- os: ubuntu-latest
5052
name: ubuntu-latest
5153
codecov-flag: ubuntu
54+
upload-coverage: true
5255
- os: windows-latest
5356
name: windows-latest
5457
codecov-flag: windows
58+
upload-coverage: false
5559
steps:
5660
- name: Checkout code
5761
uses: actions/checkout@v4
5862
- name: Setup Node.js and pnpm
5963
uses: ./.github/actions/setup-node-pnpm
60-
- name: Cache Turbo
61-
uses: actions/cache@v4
64+
- name: Restore Turbo cache
65+
id: turbo-cache
66+
uses: actions/cache/restore@v4
6267
with:
6368
path: .turbo/cache
6469
key: ${{ runner.os }}-turbo-${{ hashFiles('**/pnpm-lock.yaml') }}-${{ github.sha }}
6570
restore-keys: |
6671
${{ runner.os }}-turbo-${{ hashFiles('**/pnpm-lock.yaml') }}-
6772
${{ runner.os }}-turbo-
68-
- name: Run unit tests with coverage
69-
run: pnpm test:coverage
70-
- name: Upload coverage to Codecov
73+
- name: Run non-core coverage
74+
run: pnpm turbo run test:coverage --filter="!@roo-code/core" --log-order grouped --output-logs new-only
75+
- name: Run core unit coverage
76+
run: pnpm turbo run test:coverage:unit --filter="@roo-code/core" --log-order grouped --output-logs new-only
77+
- name: Run core integration coverage
78+
run: pnpm turbo run test:coverage:integration --filter="@roo-code/core" --log-order grouped --output-logs new-only
79+
- name: Save Turbo cache
80+
if: steps.turbo-cache.outputs.cache-hit != 'true'
81+
uses: actions/cache/save@v4
82+
with:
83+
path: .turbo/cache
84+
key: ${{ steps.turbo-cache.outputs.cache-primary-key }}
85+
# Only ubuntu uploads coverage. Windows still runs the same test
86+
# lanes for behavioral confidence, but duplicating coverage uploads
87+
# there mostly adds Codecov overhead without changing pass/fail
88+
# behavior.
89+
# Coverage is uploaded in three separate steps so each LCOV gets the
90+
# correct flag set. Codecov double-counts overlapping lines when a
91+
# single upload carries multiple flags whose paths overlap, so the
92+
# two core lanes (which both cover packages/core/src/**) must be
93+
# uploaded individually with their own lane flag.
94+
# See https://docs.codecov.com/docs/flags
95+
- name: Upload non-core coverage to Codecov
96+
if: matrix.upload-coverage
7197
uses: codecov/codecov-action@v4
7298
with:
7399
files: >-
74100
src/coverage/lcov.info,
75101
webview-ui/coverage/lcov.info,
76-
packages/core/coverage/lcov.info,
77102
packages/cloud/coverage/lcov.info,
78103
packages/telemetry/coverage/lcov.info,
79104
apps/cli/coverage/lcov.info
105+
disable_search: true
80106
flags: ${{ matrix.codecov-flag }}
81107
token: ${{ secrets.CODECOV_TOKEN }}
108+
- name: Upload core unit coverage to Codecov
109+
if: matrix.upload-coverage
110+
uses: codecov/codecov-action@v4
111+
with:
112+
files: packages/core/coverage/unit/lcov.info
113+
disable_search: true
114+
flags: ${{ matrix.codecov-flag }},core-unit
115+
token: ${{ secrets.CODECOV_TOKEN }}
116+
- name: Upload core integration coverage to Codecov
117+
if: matrix.upload-coverage
118+
uses: codecov/codecov-action@v4
119+
with:
120+
files: packages/core/coverage/integration/lcov.info
121+
disable_search: true
122+
flags: ${{ matrix.codecov-flag }},core-integration
123+
token: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/e2e.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ on:
1313
- "pnpm-lock.yaml"
1414
- "turbo.json"
1515
- ".github/actions/setup-node-pnpm/**"
16+
merge_group:
17+
types: [checks_requested]
1618

1719
jobs:
1820
e2e-mock:

codecov.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ coverage:
1111
target: 80% # new lines must be 80% covered
1212
threshold: 0%
1313

14+
flag_management:
15+
individual_flags:
16+
- name: core-unit
17+
paths:
18+
- packages/core/src/
19+
carryforward: true
20+
- name: core-integration
21+
paths:
22+
- packages/core/src/
23+
carryforward: true
24+
1425
comment:
1526
layout: "diff, flags"
1627
behavior: default

packages/core/package.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
"lint": "eslint src --ext=ts --max-warnings=0",
1313
"check-types": "tsc --noEmit",
1414
"test": "vitest run",
15-
"test:coverage": "vitest run --coverage",
15+
"test:unit": "vitest run --config vitest.unit.config.ts",
16+
"test:integration": "vitest run --config vitest.integration.config.ts",
17+
"test:coverage": "pnpm test:coverage:unit && pnpm test:coverage:integration",
18+
"test:coverage:unit": "vitest run --config vitest.unit.config.ts --coverage",
19+
"test:coverage:integration": "vitest run --config vitest.integration.config.ts --coverage",
1620
"clean": "rimraf .turbo"
1721
},
1822
"dependencies": {
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
// pnpm --filter @roo-code/core test src/custom-tools/__tests__/custom-tool-registry.integration.spec.ts
2+
3+
import path from "path"
4+
import { fileURLToPath } from "url"
5+
6+
import { CustomToolRegistry } from "../custom-tool-registry.js"
7+
8+
const __dirname = path.dirname(fileURLToPath(import.meta.url))
9+
10+
const TEST_FIXTURES_DIR = path.join(__dirname, "fixtures")
11+
const TEST_FIXTURES_OVERRIDE_DIR = path.join(__dirname, "fixtures-override")
12+
13+
describe.sequential("CustomToolRegistry integration", () => {
14+
let registry: CustomToolRegistry
15+
16+
beforeEach(() => {
17+
registry = new CustomToolRegistry()
18+
})
19+
20+
describe("loadFromDirectory", () => {
21+
it("should load tools from TypeScript files", async () => {
22+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
23+
24+
expect(result.loaded).toContain("simple")
25+
expect(registry.has("simple")).toBe(true)
26+
}, 300_000)
27+
28+
it("should handle named exports", async () => {
29+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
30+
31+
expect(result.loaded).toContain("multi_toolA")
32+
expect(result.loaded).toContain("multi_toolB")
33+
}, 30_000)
34+
35+
it("should report validation failures", async () => {
36+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
37+
38+
const invalidFailure = result.failed.find((failure) => failure.file === "invalid.ts")
39+
expect(invalidFailure).toBeDefined()
40+
expect(invalidFailure?.error).toContain("Invalid tool definition")
41+
}, 30_000)
42+
43+
it("should return empty results for non-existent directory", async () => {
44+
const result = await registry.loadFromDirectory("/nonexistent/path")
45+
46+
expect(result.loaded).toHaveLength(0)
47+
expect(result.failed).toHaveLength(0)
48+
})
49+
50+
it("should skip non-tool exports silently", async () => {
51+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
52+
53+
expect(result.loaded).toContain("mixed_validTool")
54+
expect(result.loaded).not.toContain("mixed_someString")
55+
expect(result.loaded).not.toContain("mixed_someNumber")
56+
expect(result.loaded).not.toContain("mixed_someObject")
57+
}, 30_000)
58+
59+
it("should support args as alias for parameters", async () => {
60+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
61+
62+
expect(result.loaded).toContain("legacy")
63+
64+
const tool = registry.get("legacy")
65+
expect(tool?.parameters).toBeDefined()
66+
}, 30_000)
67+
})
68+
69+
describe("clearCache", () => {
70+
it("should clear the TypeScript compilation cache", async () => {
71+
await registry.loadFromDirectory(TEST_FIXTURES_DIR)
72+
registry.clearCache()
73+
74+
registry.clear()
75+
const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR)
76+
77+
expect(result.loaded).toContain("cached")
78+
}, 300_000)
79+
})
80+
81+
describe("loadFromDirectories", () => {
82+
it("should load tools from multiple directories", async () => {
83+
const result = await registry.loadFromDirectories([TEST_FIXTURES_DIR, TEST_FIXTURES_OVERRIDE_DIR])
84+
85+
expect(result.loaded).toContain("simple")
86+
expect(result.loaded).toContain("unique_override")
87+
expect(result.loaded).toContain("multi_toolA")
88+
}, 60_000)
89+
90+
it("should allow later directories to override earlier ones", async () => {
91+
await registry.loadFromDirectories([TEST_FIXTURES_DIR, TEST_FIXTURES_OVERRIDE_DIR])
92+
93+
const simpleTool = registry.get("simple")
94+
expect(simpleTool).toBeDefined()
95+
expect(simpleTool?.description).toBe("Simple tool - OVERRIDDEN")
96+
}, 60_000)
97+
98+
it("should preserve order: first directory loaded first, second overrides", async () => {
99+
await registry.loadFromDirectories([TEST_FIXTURES_OVERRIDE_DIR, TEST_FIXTURES_DIR])
100+
101+
const simpleTool = registry.get("simple")
102+
expect(simpleTool).toBeDefined()
103+
expect(simpleTool?.description).toBe("Simple tool")
104+
}, 60_000)
105+
106+
it("should handle non-existent directories in the array", async () => {
107+
const result = await registry.loadFromDirectories([
108+
"/nonexistent/path",
109+
TEST_FIXTURES_DIR,
110+
"/another/nonexistent",
111+
])
112+
113+
expect(result.loaded).toContain("simple")
114+
expect(result.failed).toHaveLength(1)
115+
}, 60_000)
116+
117+
it("should handle empty array", async () => {
118+
const result = await registry.loadFromDirectories([])
119+
120+
expect(result.loaded).toHaveLength(0)
121+
expect(result.failed).toHaveLength(0)
122+
})
123+
124+
it("should combine results from all directories", async () => {
125+
const result = await registry.loadFromDirectories([TEST_FIXTURES_DIR, TEST_FIXTURES_OVERRIDE_DIR])
126+
127+
const simpleCount = result.loaded.filter((name) => name === "simple").length
128+
expect(simpleCount).toBe(2)
129+
}, 60_000)
130+
})
131+
132+
describe("loadFromDirectoriesIfStale", () => {
133+
it("should load tools from multiple directories when stale", async () => {
134+
const result = await registry.loadFromDirectoriesIfStale([TEST_FIXTURES_DIR, TEST_FIXTURES_OVERRIDE_DIR])
135+
136+
expect(result.loaded).toContain("simple")
137+
expect(result.loaded).toContain("unique_override")
138+
}, 60_000)
139+
140+
it("should not reload if directories are not stale", async () => {
141+
await registry.loadFromDirectoriesIfStale([TEST_FIXTURES_DIR])
142+
registry.clear()
143+
144+
const result = await registry.loadFromDirectoriesIfStale([TEST_FIXTURES_DIR])
145+
146+
expect(result.loaded).toEqual([])
147+
}, 30_000)
148+
149+
it("should handle mixed stale and non-stale directories", async () => {
150+
await registry.loadFromDirectoriesIfStale([TEST_FIXTURES_DIR])
151+
152+
const result = await registry.loadFromDirectoriesIfStale([TEST_FIXTURES_DIR, TEST_FIXTURES_OVERRIDE_DIR])
153+
154+
expect(result.loaded).toContain("simple")
155+
expect(result.loaded).toContain("unique_override")
156+
}, 60_000)
157+
})
158+
})

0 commit comments

Comments
 (0)