diff --git a/.github/workflows/update_manifests.yml b/.github/workflows/update_manifests.yml index 1d5a98dc..d9834092 100644 --- a/.github/workflows/update_manifests.yml +++ b/.github/workflows/update_manifests.yml @@ -6,6 +6,7 @@ on: permissions: contents: write + pull-requests: read jobs: update-manifests: diff --git a/src/ghw_get_changed_ota_files.ts b/src/ghw_get_changed_ota_files.ts index 3a77d2f4..7356a58c 100644 --- a/src/ghw_get_changed_ota_files.ts +++ b/src/ghw_get_changed_ota_files.ts @@ -11,7 +11,7 @@ export async function getChangedOtaFiles( core: typeof CoreApi, context: Context, basehead: string, - isPR: boolean, + throwIfFilesOutsideOfImages: boolean, ): Promise { // NOTE: includes up to 300 files, per https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#compare-two-commits const compare = await github.rest.repos.compareCommitsWithBasehead({ @@ -26,8 +26,12 @@ export async function getChangedOtaFiles( const fileList = compare.data.files.filter((f) => f.filename.startsWith(`${BASE_IMAGES_DIR}/`)); - if (isPR && fileList.length !== compare.data.files.length) { - throw new Error(`Detected changes in files outside of \`images\` directory. This is not allowed for a pull request with OTA files.`); + if (throwIfFilesOutsideOfImages && fileList.length !== compare.data.files.length) { + if (context.payload.pull_request) { + throw new Error(`Detected changes in files outside of \`images\` directory. This is not allowed for a pull request with OTA files.`); + } else { + throw new Error(`Cannot run with files outside of \`images\` directory.`); + } } return fileList.map((f) => f.filename); diff --git a/src/ghw_process_ota_files.ts b/src/ghw_process_ota_files.ts index 70519954..f77aa868 100644 --- a/src/ghw_process_ota_files.ts +++ b/src/ghw_process_ota_files.ts @@ -4,6 +4,7 @@ import type {Octokit} from '@octokit/rest'; import type {ExtraMetas, GHExtraMetas, RepoImageMeta} from './types.js'; +import assert from 'assert'; import {readFileSync, renameSync} from 'fs'; import path from 'path'; @@ -37,12 +38,40 @@ function getFileExtraMetas(extraMetas: GHExtraMetas, fileName: string): ExtraMet return extraMetas; } +async function getPRBody(github: Octokit, core: typeof CoreApi, context: Context): Promise { + assert(context.payload.pull_request || context.eventName === 'push'); + + if (context.payload.pull_request) { + return context.payload.pull_request.body; + } else if (context.eventName === 'push') { + const pushMsg = context.payload.head_commit.message as string; + const prMatch = pushMsg.match(/\(#(\d+)\)/); + + if (prMatch) { + const prNumber = parseInt(prMatch[1], 10); + + try { + const pr = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + + return pr.data.body || undefined; + } catch (error) { + throw new Error(`Failed to get PR#${prNumber} for extra metas: ${error}`); + } + } + } +} + async function parsePRBodyExtraMetas(github: Octokit, core: typeof CoreApi, context: Context): Promise { let extraMetas: GHExtraMetas = {}; - if (context.payload.pull_request?.body) { + const prBody = await getPRBody(github, core, context); + + if (prBody) { try { - const prBody = context.payload.pull_request.body; const metasStart = prBody.indexOf(EXTRA_METAS_PR_BODY_START_TAG); const metasEnd = prBody.lastIndexOf(EXTRA_METAS_PR_BODY_END_TAG); diff --git a/src/ghw_update_manifests.ts b/src/ghw_update_manifests.ts index 14e0ecf4..9cc5735f 100644 --- a/src/ghw_update_manifests.ts +++ b/src/ghw_update_manifests.ts @@ -11,7 +11,7 @@ import {processOtaFiles} from './ghw_process_ota_files.js'; export async function updateManifests(github: Octokit, core: typeof CoreApi, context: Context): Promise { assert(context.eventName === 'push', 'Not a push'); - const filePaths = await getChangedOtaFiles(github, core, context, `${context.payload.before}...${context.payload.after}`, false); + const filePaths = await getChangedOtaFiles(github, core, context, `${context.payload.before}...${context.payload.after}`, true); const baseManifest = readManifest(BASE_INDEX_MANIFEST_FILENAME); const prevManifest = readManifest(PREV_INDEX_MANIFEST_FILENAME); diff --git a/tests/ghw_update_manifests.test.ts b/tests/ghw_update_manifests.test.ts new file mode 100644 index 00000000..3c0389ea --- /dev/null +++ b/tests/ghw_update_manifests.test.ts @@ -0,0 +1,206 @@ +import type CoreApi from '@actions/core'; +import type {Context} from '@actions/github/lib/context'; +import type {Octokit} from '@octokit/rest'; + +import type {RepoImageMeta} from '../src/types'; + +import {rmSync} from 'fs'; + +import * as common from '../src/common'; +import {updateManifests} from '../src/ghw_update_manifests'; +import { + BASE_IMAGES_TEST_DIR_PATH, + getAdjustedContent, + IMAGE_V13_1, + IMAGE_V14_1, + IMAGE_V14_1_METAS, + PREV_IMAGES_TEST_DIR_PATH, + useImage, + withExtraMetas, +} from './data.test'; + +const github = { + rest: { + pulls: { + get: jest.fn, Parameters, unknown>(), + }, + repos: { + compareCommitsWithBasehead: jest.fn< + ReturnType, + Parameters, + unknown + >(), + }, + }, +}; +const core: Partial = { + debug: console.debug, + info: console.log, + warning: console.warn, + error: console.error, + notice: console.log, + startGroup: jest.fn(), + endGroup: jest.fn(), +}; +const context: Partial = { + eventName: 'push', + payload: { + head_commit: { + message: 'push from pr (#213)', + }, + }, + repo: { + owner: 'Koenkk', + repo: 'zigbee-OTA', + }, +}; + +describe('Github Workflow: Update manifests', () => { + let baseManifest: RepoImageMeta[]; + let prevManifest: RepoImageMeta[]; + let readManifestSpy: jest.SpyInstance; + let writeManifestSpy: jest.SpyInstance; + let addImageToBaseSpy: jest.SpyInstance; + let addImageToPrevSpy: jest.SpyInstance; + let filePaths: ReturnType[] = []; + let prBody: string | undefined; + + const getManifest = (fileName: string): RepoImageMeta[] => { + if (fileName === common.BASE_INDEX_MANIFEST_FILENAME) { + return baseManifest; + } else if (fileName === common.PREV_INDEX_MANIFEST_FILENAME) { + return prevManifest; + } else { + throw new Error(`${fileName} not supported`); + } + }; + + const setManifest = (fileName: string, content: RepoImageMeta[]): void => { + const adjustedContent = getAdjustedContent(fileName, content); + + if (fileName === common.BASE_INDEX_MANIFEST_FILENAME) { + baseManifest = adjustedContent; + } else if (fileName === common.PREV_INDEX_MANIFEST_FILENAME) { + prevManifest = adjustedContent; + } else { + throw new Error(`${fileName} not supported`); + } + }; + + const resetManifests = (): void => { + baseManifest = []; + prevManifest = []; + }; + + const expectNoChanges = (noReadManifest: boolean = false): void => { + if (noReadManifest) { + expect(readManifestSpy).toHaveBeenCalledTimes(0); + } else { + expect(readManifestSpy).toHaveBeenCalledTimes(2); + } + + expect(addImageToBaseSpy).toHaveBeenCalledTimes(0); + expect(addImageToPrevSpy).toHaveBeenCalledTimes(0); + expect(writeManifestSpy).toHaveBeenCalledTimes(0); + }; + + beforeAll(() => {}); + + afterAll(() => { + readManifestSpy.mockRestore(); + writeManifestSpy.mockRestore(); + addImageToBaseSpy.mockRestore(); + addImageToPrevSpy.mockRestore(); + rmSync(BASE_IMAGES_TEST_DIR_PATH, {recursive: true, force: true}); + rmSync(PREV_IMAGES_TEST_DIR_PATH, {recursive: true, force: true}); + }); + + beforeEach(() => { + resetManifests(); + + filePaths = []; + readManifestSpy = jest.spyOn(common, 'readManifest').mockImplementation(getManifest); + writeManifestSpy = jest.spyOn(common, 'writeManifest').mockImplementation(setManifest); + addImageToBaseSpy = jest.spyOn(common, 'addImageToBase'); + addImageToPrevSpy = jest.spyOn(common, 'addImageToPrev'); + github.rest.pulls.get.mockImplementation( + // @ts-expect-error mock + () => ({data: {body: prBody}}), + ); + github.rest.repos.compareCommitsWithBasehead.mockImplementation( + // @ts-expect-error mock + () => ({data: {files: filePaths}}), + ); + }); + + afterEach(() => { + rmSync(BASE_IMAGES_TEST_DIR_PATH, {recursive: true, force: true}); + rmSync(PREV_IMAGES_TEST_DIR_PATH, {recursive: true, force: true}); + rmSync(common.PR_ARTIFACT_DIR, {recursive: true, force: true}); + }); + + it('hard failure from outside push context', async () => { + filePaths = [useImage(IMAGE_V14_1)]; + + await expect(async () => { + // @ts-expect-error mock + await updateManifests(github, core, {payload: {}}); + }).rejects.toThrow(`Not a push`); + + expectNoChanges(true); + }); + + it('failure with file outside of images directory', async () => { + filePaths = [useImage(IMAGE_V13_1, PREV_IMAGES_TEST_DIR_PATH), useImage(IMAGE_V14_1)]; + + await expect(async () => { + // @ts-expect-error mock + await updateManifests(github, core, context); + }).rejects.toThrow(expect.objectContaining({message: expect.stringContaining(`Cannot run with files outside`)})); + + expectNoChanges(true); + }); + + it('success into base', async () => { + filePaths = [useImage(IMAGE_V14_1)]; + + // @ts-expect-error mock + await updateManifests(github, core, context); + + expect(readManifestSpy).toHaveBeenCalledWith(common.BASE_INDEX_MANIFEST_FILENAME); + expect(readManifestSpy).toHaveBeenCalledWith(common.PREV_INDEX_MANIFEST_FILENAME); + expect(addImageToBaseSpy).toHaveBeenCalledTimes(1); + expect(addImageToPrevSpy).toHaveBeenCalledTimes(0); + expect(writeManifestSpy).toHaveBeenCalledTimes(2); + expect(writeManifestSpy).toHaveBeenCalledWith(common.BASE_INDEX_MANIFEST_FILENAME, [IMAGE_V14_1_METAS]); + }); + + it('success with extra metas', async () => { + filePaths = [useImage(IMAGE_V14_1)]; + prBody = `Text before start tag \`\`\`json {"manufacturerName": ["lixee"]} \`\`\` Text after end tag`; + + // @ts-expect-error mock + await updateManifests(github, core, context); + + expect(readManifestSpy).toHaveBeenCalledWith(common.BASE_INDEX_MANIFEST_FILENAME); + expect(readManifestSpy).toHaveBeenCalledWith(common.PREV_INDEX_MANIFEST_FILENAME); + expect(addImageToBaseSpy).toHaveBeenCalledTimes(1); + expect(addImageToPrevSpy).toHaveBeenCalledTimes(0); + expect(writeManifestSpy).toHaveBeenCalledTimes(2); + expect(writeManifestSpy).toHaveBeenCalledWith(common.BASE_INDEX_MANIFEST_FILENAME, [ + withExtraMetas(IMAGE_V14_1_METAS, {manufacturerName: ['lixee']}), + ]); + }); + + it('fails to get PR for extra metas', async () => { + filePaths = [useImage(IMAGE_V14_1)]; + github.rest.pulls.get.mockRejectedValueOnce('403'); + + await expect(async () => { + // @ts-expect-error mock + await updateManifests(github, core, context); + }).rejects.toThrow(expect.objectContaining({message: `Failed to get PR#213 for extra metas: 403`})); + + expectNoChanges(false); + }); +});