Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ on:
branches: [main]
push:
branches: [main]
schedule:
- cron: '43 9 * * *'
workflow_dispatch:

concurrency:
group: ci-${{ github.workflow }}-${{ github.ref }}
group: ci-${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

permissions:
Expand Down Expand Up @@ -63,7 +65,7 @@ jobs:
# PR checks may predate later main changes, so the planning lane also
# validates the exact merged delta. Dispatches and unavailable history
# fail safe to every surface.
if [[ "${{ github.event_name }}" == "workflow_dispatch" ]] || [[ "$BASE_SHA" =~ ^0+$ ]] || ! git cat-file -e "${BASE_SHA}^{commit}"; then
if [[ "${{ github.event_name }}" == "workflow_dispatch" || "${{ github.event_name }}" == "schedule" ]] || [[ "$BASE_SHA" =~ ^0+$ ]] || ! git cat-file -e "${BASE_SHA}^{commit}"; then
node scripts/ci-test-plan.mjs --full >> "$GITHUB_OUTPUT"
else
node scripts/ci-test-plan.mjs --base "$BASE_SHA" --head "$HEAD_SHA" >> "$GITHUB_OUTPUT"
Expand Down Expand Up @@ -174,6 +176,10 @@ jobs:
if: steps.plan.outputs.code == 'true' || steps.plan.outputs.astryx_surface == 'true' || steps.plan.outputs.asf_source == 'true' || steps.plan.outputs.cli_package == 'true' || steps.plan.outputs.release_contract == 'true'
run: npm ci

- name: Test Desktop e2e selection
if: steps.plan.outputs.e2e == 'true'
run: node --test scripts/desktop-e2e-test-selection.test.mjs

- name: Check localized TUI copy boundaries
if: steps.plan.outputs.code == 'true'
run: |
Expand Down Expand Up @@ -336,8 +342,23 @@ jobs:

- name: Desktop e2e
if: steps.plan.outputs.e2e == 'true'
env:
BASE_SHA: ${{ github.event_name == 'push' && github.event.before || github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event_name == 'push' && github.sha || github.event.pull_request.head.sha }}
run: |
set -euo pipefail
spec_list="$RUNNER_TEMP/desktop-e2e-specs.txt"
if [[ "${{ steps.plan.outputs.full }}" == "true" ]]; then
node scripts/desktop-e2e-test-selection.mjs --full > "$spec_list"
else
node scripts/desktop-e2e-test-selection.mjs \
--base "$BASE_SHA" --head "$HEAD_SHA" > "$spec_list"
fi
mapfile -t specs < "$spec_list"
if (( ${#specs[@]} == 0 )); then
echo "No Desktop e2e spec is reachable from this change"
exit 0
fi
display_base=90
worker_count=4
xvfb_pids=()
Expand Down Expand Up @@ -366,7 +387,7 @@ jobs:
done
MAKA_E2E_X_DISPLAY_BASE="$display_base" \
npm exec -w @maka/desktop -- playwright test \
--config e2e/playwright.config.ts --workers="$worker_count"
--config e2e/playwright.config.ts --workers="$worker_count" "${specs[@]}"

- name: Browser WebContentsView semantic smoke
if: steps.plan.outputs.e2e == 'true'
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/e2e/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default defineConfig({
testDir: '.',
fullyParallel: true,
workers: 1,
reporter: 'list',
// CI publishes no Playwright report that consumes Git metadata. Disable its
// best-effort shallow-history fetch, which otherwise waits on a fixed timeout.
captureGitInfo: { commit: false, diff: false },
Expand Down
7 changes: 7 additions & 0 deletions scripts/ci-test-plan.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ const RELEASE_CONTRACT_FILES = new Set([
'scripts/windows-upgrade-baseline.json',
'scripts/windows-package-source-closure.mjs',
'scripts/windows-package-source-closure.test.mjs',
'scripts/workspace-source-closure.mjs',
// Reads the filter that closure test compares against, and `check:release`
// is the only gate that runs it against `release-windows-check.yml`.
'scripts/workflow-pull-request-paths.mjs',
Expand Down Expand Up @@ -204,6 +205,11 @@ const E2E_DRIVING_SCRIPTS = new Set([
'apps/desktop/scripts/browser-observe-act-smoke.mjs',
'scripts/audit-alignment.mjs',
'scripts/ax-tree-audit.mjs',
'scripts/desktop-e2e-test-selection.mjs',
'scripts/desktop-e2e-test-selection.test.mjs',
'scripts/electron-lifecycle.mjs',
'scripts/fixture-env.mjs',
'scripts/workspace-source-closure.mjs',
]);

// Scripts / paths that can break the built Storybook catalog. Product stories
Expand Down Expand Up @@ -542,6 +548,7 @@ export function formatGitHubOutputs(plan) {
`cli_package=${plan.cliPackage}`,
`code=${plan.code}`,
`e2e=${plan.e2e}`,
`full=${plan.full}`,
`runtime_host=${plan.runtimeHost}`,
`runtime_sandbox=${plan.runtimeSandbox}`,
`release_contract=${plan.releaseContract}`,
Expand Down
23 changes: 22 additions & 1 deletion scripts/ci-workflow-policy.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,20 @@ test('planning runs first and every later step gates on its outputs', () => {
);
});

test('core CI validates pull requests and the resulting main branch state', () => {
test('core CI validates pull requests, the resulting main branch state, and a nightly full run', () => {
const workflow = readWorkflow('ci.yml');

assert.match(workflow, /pull_request:\n\s+branches: \[main\]/u);
assert.match(workflow, /push:\n\s+branches: \[main\]/u);
assert.match(workflow, /schedule:\n\s+- cron:/u);
assert.match(
workflow,
/group: ci-\$\{\{ github\.workflow \}\}-\$\{\{ github\.event_name \}\}-\$\{\{ github\.ref \}\}/u,
);
assert.match(
workflow,
/github\.event_name \}\}" == "workflow_dispatch" \|\| "\$\{\{ github\.event_name \}\}" == "schedule"/u,
);
assert.match(
workflow,
/BASE_SHA: \$\{\{ github\.event_name == 'push' && github\.event\.before \|\| github\.event\.pull_request\.base\.sha \}\}/u,
Expand All @@ -99,6 +108,18 @@ test('core CI validates pull requests and the resulting main branch state', () =
/HEAD_SHA: \$\{\{ github\.event_name == 'push' && github\.sha \|\| github\.event\.pull_request\.head\.sha \}\}/u,
);
assert.match(workflow, /\[\[ "\$BASE_SHA" =~ \^0\+\$ \]\]/u);
assert.match(
workflow,
/if \[\[ "\$\{\{ steps\.plan\.outputs\.full \}\}" == "true" \]\]; then\n\s+node scripts\/desktop-e2e-test-selection\.mjs --full/u,
);
assert.match(
workflow,
/mapfile -t specs < "\$spec_list"\n\s+if \(\( \$\{#specs\[@\]\} == 0 \)\); then\n\s+echo "No Desktop e2e spec is reachable from this change"\n\s+exit 0\n\s+fi\n\s+display_base=90/u,
);
assert.match(
workflow,
/--config e2e\/playwright\.config\.ts --workers="\$worker_count" "\$\{specs\[@\]\}"/u,
);
});

test('core CI uses the Windows inventory package-script authority', () => {
Expand Down
101 changes: 101 additions & 0 deletions scripts/desktop-e2e-test-selection.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#!/usr/bin/env node
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { readdirSync } from 'node:fs';
import { dirname, relative, resolve, sep } from 'node:path';
import { fileURLToPath } from 'node:url';

import { changedFilesBetween, planTests } from './ci-test-plan.mjs';
import { collectWorkspaceSourceClosure } from './workspace-source-closure.mjs';

const scriptPath = fileURLToPath(import.meta.url);
const defaultRepoRoot = dirname(dirname(scriptPath));
const desktopE2eRoot = 'apps/desktop/e2e';

const SELECTION_AUTHORITY_FILES = new Set([
'apps/desktop/e2e/playwright.config.ts',
'scripts/desktop-e2e-test-selection.mjs',
'scripts/workspace-source-closure.mjs',
]);

function normalizePath(path) {
return path.split(sep).join('/').replace(/^\.\//u, '');
}

export function listDesktopE2eSpecs(repoRoot = defaultRepoRoot) {
const root = resolve(repoRoot, desktopE2eRoot);
return readdirSync(root, { recursive: true })
.filter((path) => path.endsWith('.spec.ts'))
.map((path) => normalizePath(relative(repoRoot, resolve(root, path))))
.sort();
}

export async function collectDesktopE2eSpecClosures(specs, repoRoot = defaultRepoRoot) {
const closures = new Map();
for (const spec of specs) {
closures.set(spec, new Set(await collectWorkspaceSourceClosure([spec], repoRoot)));
}
return closures;
}

export async function selectDesktopE2eSpecs(changedFiles, options = {}) {
const repoRoot = options.repoRoot ?? defaultRepoRoot;
const files = [...new Set(changedFiles.map(normalizePath).filter(Boolean))];
const specs = options.specs ?? listDesktopE2eSpecs(repoRoot);
const plan = planTests(files, { repoRoot, forceFull: options.forceFull });
if (!plan.e2e) return [];

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.

P3: Redundant with the caller. The Desktop e2e step is already gated on steps.plan.outputs.e2e and already branches on steps.plan.outputs.full, so plan.e2e, plan.full and the second changedFilesBetween here recompute a decision the same job made one step earlier. Let the selector take changed files and nothing else.

if (plan.full || files.some((path) => SELECTION_AUTHORITY_FILES.has(path))) return specs;

const closures = options.closures ?? (await collectDesktopE2eSpecClosures(specs, repoRoot));
return specs.filter((spec) => files.some((path) => closures.get(spec)?.has(path)));

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.

P1: This is the whole selection, and the relation it uses is wrong for e2e. A spec imports ./fixtures and then launches a built Electron app, so the app under test never appears in the spec's static closure. Measured at this head against the files that make the planner set e2e=true: apps/desktop/src/main 1 of 190 files reach any spec, src/renderer 29 of 493, src/preload 1 of 16, src/shared 1 of 8. Replaying the last 60 first-parent commits, 11 of the 39 e2e-triggering merges select zero specs, including e1e108a79, which rewrote src/renderer/settings/settings-surface.tsx while settings.spec.ts and settings-row-focus-ring.spec.ts drive that exact surface. The gate goes green with nothing run and the nightly catches it after merge. The smallest correct fix (adding the desktop main/preload/renderer entry points to every spec's closure) also removes essentially all of the reduction, which is the signal that static import reachability cannot select e2e specs. Selection needs coverage declared per spec or sampled from an instrumented run.

}

function parseArgs(args) {
const parsed = { base: undefined, forceFull: false, head: undefined };
for (let index = 0; index < args.length; index += 1) {
const arg = args[index];
if (arg === '--full') parsed.forceFull = true;
else if (arg === '--base') parsed.base = args[++index];
else if (arg === '--head') parsed.head = args[++index];
else throw new Error(`Unknown argument: ${arg}`);
}
if (!parsed.forceFull && (!parsed.base || !parsed.head)) {
throw new Error('Expected --full or both --base <sha> and --head <sha>');
}
return parsed;
}

async function main(args) {
const parsed = parseArgs(args);
const changedFiles = parsed.forceFull ? [] : changedFilesBetween(parsed.base, parsed.head);
const specs = await selectDesktopE2eSpecs(changedFiles, { forceFull: parsed.forceFull });
const workspaceSpecs = specs.map((path) => path.slice('apps/desktop/'.length));
process.stdout.write(workspaceSpecs.length > 0 ? `${workspaceSpecs.join('\n')}\n` : '');
process.stderr.write(`Desktop e2e selection: ${specs.length} spec files\n`);
}

if (process.argv[1] && resolve(process.argv[1]) === scriptPath) {
try {
await main(process.argv.slice(2));
} catch (error) {
process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`);
process.exitCode = 2;
}
}
104 changes: 104 additions & 0 deletions scripts/desktop-e2e-test-selection.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import assert from 'node:assert/strict';
import test from 'node:test';

import {
collectDesktopE2eSpecClosures,
listDesktopE2eSpecs,
selectDesktopE2eSpecs,
} from './desktop-e2e-test-selection.mjs';

const specs = ['apps/desktop/e2e/alpha.spec.ts', 'apps/desktop/e2e/beta.spec.ts'];
const closures = new Map([
[specs[0], new Set([specs[0], 'apps/desktop/e2e/fixtures.ts', 'apps/desktop/src/main/a.ts'])],
[specs[1], new Set([specs[1], 'apps/desktop/e2e/fixtures.ts', 'apps/desktop/src/main/b.ts'])],
]);

test('selection follows the exact source closure of each spec', async () => {
assert.deepEqual(
await selectDesktopE2eSpecs(['apps/desktop/src/main/a.ts'], { specs, closures }),
[specs[0]],
);
assert.deepEqual(
await selectDesktopE2eSpecs(['apps/desktop/e2e/fixtures.ts'], { specs, closures }),
specs,
);
assert.deepEqual(
await selectDesktopE2eSpecs(['apps/desktop/src/main/unreached.ts'], { specs, closures }),
[],
);
});

test('full plans and selection authorities run every spec', async () => {
for (const path of [
'package-lock.json',
'apps/desktop/e2e/playwright.config.ts',
'scripts/desktop-e2e-test-selection.mjs',
'scripts/workspace-source-closure.mjs',
]) {
assert.deepEqual(await selectDesktopE2eSpecs([path], { specs, closures }), specs, path);
}
});

test('non-Electron changes select no Desktop spec', async () => {
assert.deepEqual(
await selectDesktopE2eSpecs(['packages/runtime/src/runtime.ts'], { specs, closures }),
[],
);
});

test('current spec closures select every repository script they import', async () => {
const currentSpecs = listDesktopE2eSpecs();
const currentClosures = await collectDesktopE2eSpecClosures(currentSpecs);

assert.ok(currentSpecs.length > 0);
for (const spec of currentSpecs) {
assert.ok(currentClosures.get(spec)?.has(spec), spec);
assert.ok(currentClosures.get(spec)?.has('apps/desktop/e2e/fixtures.ts'), spec);

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.

P2: The suite does not test the scenario the PR is for. Three of the four tests use the hand-written closures fixture and only prove the filter; this one uses real closures but asserts only scripts/ reachability and that every spec closure contains fixtures.ts. Nothing asserts anything about apps/desktop/src/**, which is where the selection actually fails. Add an assertion that every product source file under apps/desktop/src selects at least one spec. It fails today, and that failure is the evidence the design needs.

}

const scriptInputs = [
...new Set(
[...currentClosures.values()].flatMap((closure) =>
[...closure].filter((path) => path.startsWith('scripts/')),
),
),
].sort();
assert.ok(scriptInputs.length > 0);
for (const path of scriptInputs) {
const reachableSpecs = currentSpecs.filter((spec) => currentClosures.get(spec)?.has(path));
assert.deepEqual(
await selectDesktopE2eSpecs([path], {
specs: currentSpecs,
closures: currentClosures,
}),
reachableSpecs,
path,
);
}
assert.deepEqual(
await selectDesktopE2eSpecs(['scripts/ax-tree-audit.mjs'], {
specs: currentSpecs,
closures: currentClosures,
}),
['apps/desktop/e2e/accessibility-coverage.spec.ts'],
);
});
Loading
Loading