-
Notifications
You must be signed in to change notification settings - Fork 18
fix(ci): prepare macOS resources before optional signing #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b4e184a
c990ac9
17e9d24
fb4bab4
3d3fbd3
5468c06
943ef99
b664faa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
| import { | ||
| extractWorkflowJobSteps, | ||
| findStep, | ||
| findStepIndex, | ||
| readWorkflowObject, | ||
| } from './workflow-test-utils.mjs'; | ||
|
|
||
| const WORKFLOW_FILE = 'build-desktop-tauri.yml'; | ||
| const BUILD_MACOS_JOB = 'build-macos'; | ||
| const PREPARE_RESOURCES_RUN = /pnpm run prepare:resources/; | ||
| const PRESIGN_BACKEND_RUN = /codesign-macos-nested\.sh\s+"resources\/backend"/; | ||
| const BUILD_APP_BUNDLE_RUN = /cargo tauri build --verbose --target/; | ||
|
|
||
| test('findStep supports predicate and regex matching', () => { | ||
| const steps = [ | ||
| { name: 'Prepare desktop resources (macOS) [unsigned-compatible]', run: 'pnpm run prepare:resources' }, | ||
| { name: 'Build desktop app bundle (macOS) release artifacts', run: 'cargo tauri build --verbose --target x86_64-apple-darwin' }, | ||
| ]; | ||
|
|
||
| assert.equal(findStep(steps, 'prepare resources run', (step) => PREPARE_RESOURCES_RUN.test(step.run ?? '')), steps[0]); | ||
| assert.equal(findStep(steps, /Build desktop app bundle/, (step) => BUILD_APP_BUNDLE_RUN.test(step.run ?? '')), steps[1]); | ||
| }); | ||
|
|
||
| test('macOS workflow exposes structured build-macos steps', async () => { | ||
| const workflowObject = await readWorkflowObject(WORKFLOW_FILE); | ||
| const steps = extractWorkflowJobSteps(workflowObject, BUILD_MACOS_JOB); | ||
|
|
||
| assert.ok(findStep(steps, 'prepare resources step', (step) => PREPARE_RESOURCES_RUN.test(step.run ?? ''))); | ||
| assert.ok(findStep(steps, 'pre-sign resources step', (step) => PRESIGN_BACKEND_RUN.test(step.run ?? ''))); | ||
| assert.ok(findStep(steps, 'build app bundle step', (step) => BUILD_APP_BUNDLE_RUN.test(step.run ?? ''))); | ||
| }); | ||
|
|
||
| test('macOS workflow prepares resources before optional pre-signing', async () => { | ||
| const workflowObject = await readWorkflowObject(WORKFLOW_FILE); | ||
| const steps = extractWorkflowJobSteps(workflowObject, BUILD_MACOS_JOB); | ||
| const prepareStepIndex = findStepIndex( | ||
| steps, | ||
| (step) => PREPARE_RESOURCES_RUN.test(step.run ?? ''), | ||
| 'prepare resources step', | ||
| ); | ||
| const preSignStepIndex = findStepIndex( | ||
| steps, | ||
| (step) => PRESIGN_BACKEND_RUN.test(step.run ?? ''), | ||
| 'pre-sign resources step', | ||
| ); | ||
| const buildStepIndex = findStepIndex( | ||
| steps, | ||
| (step) => BUILD_APP_BUNDLE_RUN.test(step.run ?? ''), | ||
| 'build app bundle step', | ||
| ); | ||
| const prepareStep = steps[prepareStepIndex]; | ||
| const preSignStep = steps[preSignStepIndex]; | ||
| const buildStep = steps[buildStepIndex]; | ||
|
|
||
| assert.equal(prepareStep.if, undefined); | ||
| assert.match(prepareStep.run, /pnpm run prepare:resources/); | ||
| assert.match(prepareStep.run, /resources\/backend not found after prepare:resources/); | ||
|
|
||
| assert.match(preSignStep.if ?? '', /import_apple_certificate\.outputs\.signing_identity/); | ||
| assert.doesNotMatch(preSignStep.run, /pnpm run prepare:resources/); | ||
|
|
||
| assert.ok(prepareStepIndex < preSignStepIndex); | ||
| assert.ok(preSignStepIndex < buildStepIndex); | ||
| assert.match( | ||
| buildStep.run, | ||
| /Resources are already prepared/, | ||
| ); | ||
|
Comment on lines
+66
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current regex-based assertions are somewhat fragile because Consider using const prepareIdx = workflow.indexOf('- name: Prepare desktop resources (macOS)');
const presignIdx = workflow.indexOf('- name: Pre-sign backend resources (macOS)');
const buildIdx = workflow.indexOf('Build desktop app bundle (macOS)');
assert.ok(prepareIdx !== -1 && presignIdx !== -1 && buildIdx !== -1, 'Required workflow steps not found');
assert.ok(prepareIdx < presignIdx, 'Prepare step should come before Pre-sign step');
assert.ok(presignIdx < buildIdx, 'Pre-sign step should come before Build step');
assert.match(
workflow.slice(prepareIdx, presignIdx),
/run: \|[\s\S]*?pnpm run prepare:resources/,
);
assert.doesNotMatch(
workflow.slice(presignIdx, buildIdx),
/pnpm run prepare:resources/,
);
assert.match(
workflow.slice(buildIdx),
/# Resources are already prepared and, when available, pre-signed in earlier steps\./,
); |
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
| import { | ||
| extractWorkflowJobSteps, | ||
| findStep, | ||
| findStepIndex, | ||
| readWorkflowObject, | ||
| } from './workflow-test-utils.mjs'; | ||
|
|
||
| const WORKFLOW_FILE = 'check-scripts.yml'; | ||
| const SCRIPTS_JOB = 'scripts'; | ||
|
|
||
| test('check-scripts workflow installs node dependencies before running node tests', async () => { | ||
| const workflowObject = await readWorkflowObject(WORKFLOW_FILE); | ||
| const steps = extractWorkflowJobSteps(workflowObject, SCRIPTS_JOB); | ||
| const setupToolchainsStep = findStep( | ||
| steps, | ||
| 'setup toolchains step', | ||
| (step) => (step.uses ?? '').includes('./.github/actions/setup-toolchains'), | ||
| ); | ||
| const installStep = findStep( | ||
| steps, | ||
| 'installing node dependencies', | ||
| (step) => /pnpm install/.test(step.run ?? ''), | ||
| ); | ||
|
|
||
| const pnpmSetupIndex = findStepIndex( | ||
| steps, | ||
| (step) => (step.uses ?? '').includes('pnpm/action-setup'), | ||
| 'using pnpm/action-setup', | ||
| ); | ||
| const setupToolchainsIndex = findStepIndex( | ||
| steps, | ||
| (step) => step === setupToolchainsStep, | ||
| 'using setup toolchains', | ||
| ); | ||
| const installIndex = findStepIndex( | ||
| steps, | ||
| (step) => step === installStep, | ||
| 'installing node dependencies', | ||
| ); | ||
| const nodeTestIndex = findStepIndex( | ||
| steps, | ||
| (step) => /node --test/.test(step.run ?? ''), | ||
| 'running Node script behavior tests', | ||
| ); | ||
|
|
||
| assert.equal(setupToolchainsStep.with['node-cache'], 'pnpm'); | ||
| assert.equal(setupToolchainsStep.with['node-cache-dependency-path'], 'pnpm-lock.yaml'); | ||
| assert.match(installStep.run, /pnpm install/); | ||
| assert.match(installStep.run, /--frozen-lockfile/); | ||
| assert.match(installStep.run, /--ignore-scripts/); | ||
|
|
||
| assert.ok(pnpmSetupIndex < setupToolchainsIndex); | ||
| assert.ok(pnpmSetupIndex < installIndex); | ||
| assert.ok(installIndex < nodeTestIndex); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
| import { | ||
| extractCompositeActionSteps, | ||
| findStep, | ||
| readActionObject, | ||
| } from './workflow-test-utils.mjs'; | ||
|
|
||
| const ACTION_DIR = 'setup-toolchains'; | ||
|
|
||
| test('setup-toolchains action forwards optional node cache settings to setup-node', async () => { | ||
| const actionObject = await readActionObject(ACTION_DIR); | ||
| const steps = extractCompositeActionSteps(actionObject, ACTION_DIR); | ||
| const setupNodeStep = findStep( | ||
| steps, | ||
| 'setup-node step', | ||
| (step) => (step.uses ?? '').includes('actions/setup-node'), | ||
| ); | ||
|
|
||
| assert.equal(actionObject.inputs['node-cache'].default, ''); | ||
| assert.equal(actionObject.inputs['node-cache-dependency-path'].default, ''); | ||
| assert.equal(setupNodeStep.with.cache, '${{ inputs.node-cache }}'); | ||
| assert.equal( | ||
| setupNodeStep.with['cache-dependency-path'], | ||
| '${{ inputs.node-cache-dependency-path }}', | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import path from 'node:path'; | ||
| import { readFile } from 'node:fs/promises'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { parse } from 'yaml'; | ||
|
|
||
| const scriptDir = path.dirname(fileURLToPath(import.meta.url)); | ||
| const projectRoot = path.resolve(scriptDir, '..', '..'); | ||
| const workflowsDir = path.join(projectRoot, '.github', 'workflows'); | ||
| const actionsDir = path.join(projectRoot, '.github', 'actions'); | ||
|
|
||
| const readYamlObject = async (targetPath) => { | ||
| const content = await readFile(targetPath, 'utf8'); | ||
| return parse(content); | ||
| }; | ||
|
|
||
| export const readWorkflowObject = async (workflowFileName) => { | ||
| const workflowPath = path.join(workflowsDir, workflowFileName); | ||
| return readYamlObject(workflowPath); | ||
| }; | ||
|
|
||
| export const readActionObject = async (actionDirName) => { | ||
| const actionPath = path.join(actionsDir, actionDirName, 'action.yml'); | ||
| return readYamlObject(actionPath); | ||
| }; | ||
|
|
||
| export const extractWorkflowJobSteps = (workflowObject, jobName) => { | ||
| assert.ok(workflowObject.jobs, 'Expected workflow to define jobs.'); | ||
| const job = workflowObject.jobs[jobName]; | ||
| assert.ok(job, `Expected workflow job ${jobName} to exist.`); | ||
| assert.ok(Array.isArray(job.steps), `Expected workflow job ${jobName} to define steps.`); | ||
| return job.steps; | ||
| }; | ||
|
|
||
| export const findStep = (steps, label, predicate) => { | ||
| const step = steps.find(predicate); | ||
| assert.ok(step, `Expected workflow step ${String(label)} to exist.`); | ||
| return step; | ||
| }; | ||
|
|
||
| export const findStepIndex = (steps, predicate, label) => { | ||
| const index = steps.findIndex(predicate); | ||
| assert.notEqual(index, -1, `Expected workflow step ${String(label)} to exist.`); | ||
| return index; | ||
| }; | ||
|
|
||
| export const extractCompositeActionSteps = (actionObject, actionLabel) => { | ||
| assert.equal( | ||
| actionObject.runs?.using, | ||
| 'composite', | ||
| `Expected action ${actionLabel} to be a composite action.`, | ||
| ); | ||
| assert.ok( | ||
| Array.isArray(actionObject.runs?.steps), | ||
| `Expected action ${actionLabel} to define composite steps.`, | ||
| ); | ||
| return actionObject.runs.steps; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Align shell configuration for steps that rely on bash-specific options like
pipefailThis step uses
set -euo pipefailbut doesn’t declareshell: bash, while the following "Pre-sign backend resources" step does for the same pattern. Please also setshell: bashhere to avoid dependence on the current macOS default and keep the related steps consistent.Suggested implementation:
From the snippet, it looks like the
run: |block for "Prepare desktop resources (macOS)" is omitted or partially shown. Ensure that this step’s script (the part that usesset -euo pipefail) is indeed under this step and remains unchanged, now executing under bash due to the newshell: bashline.