diff --git a/.github/workflows/test-meticulous-against-deployment.yaml b/.github/workflows/test-meticulous-against-deployment.yaml index c677afb0..bf2060af 100644 --- a/.github/workflows/test-meticulous-against-deployment.yaml +++ b/.github/workflows/test-meticulous-against-deployment.yaml @@ -9,6 +9,7 @@ on: pull_request: {} permissions: + actions: write contents: read statuses: write pull-requests: write diff --git a/.github/workflows/test-meticulous.yaml b/.github/workflows/test-meticulous.yaml index 5249ba88..411b707d 100644 --- a/.github/workflows/test-meticulous.yaml +++ b/.github/workflows/test-meticulous.yaml @@ -9,6 +9,7 @@ on: pull_request: {} permissions: + actions: write contents: read statuses: write pull-requests: write diff --git a/src/action.ts b/src/action.ts index 8bd467ac..ba0eb949 100644 --- a/src/action.ts +++ b/src/action.ts @@ -1,7 +1,10 @@ import { setFailed } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { applyDefaultExecutionOptionsFromProject } from "@alwaysmeticulous/client"; -import { setMeticulousLocalDataDir } from "@alwaysmeticulous/common"; +import { + METICULOUS_LOGGER_NAME, + setMeticulousLocalDataDir, +} from "@alwaysmeticulous/common"; import { executeTestRun } from "@alwaysmeticulous/replay-orchestrator-launcher"; import { ReplayExecutionOptions, @@ -9,6 +12,7 @@ import { } from "@alwaysmeticulous/sdk-bundles-api"; import { initSentry } from "@alwaysmeticulous/sentry"; import debounce from "lodash.debounce"; +import log from "loglevel"; import { addLocalhostAliases } from "./utils/add-localhost-aliases"; import { throwIfCannotConnectToOrigin } from "./utils/check-connection"; import { LOGICAL_ENVIRONMENT_VERSION } from "./utils/constants"; @@ -17,7 +21,7 @@ import { getEnvironment } from "./utils/environment.utils"; import { getBaseAndHeadCommitShas } from "./utils/get-base-and-head-commit-shas"; import { getCodeChangeEvent } from "./utils/get-code-change-event"; import { getInputs } from "./utils/get-inputs"; -import { initLogger, setLogLevel } from "./utils/logger.utils"; +import { initLogger, setLogLevel, shortSha } from "./utils/logger.utils"; import { ResultsReporter } from "./utils/results-reporter"; import { waitForDeploymentUrl } from "./utils/wait-for-deployment-url"; @@ -73,9 +77,10 @@ export const runMeticulousTestsAction = async (): Promise => { const event = getCodeChangeEvent(context.eventName, payload); const { owner, repo } = context.repo; const octokit = getOctokitOrFail(githubToken); + const logger = log.getLogger(METICULOUS_LOGGER_NAME); if (event == null) { - console.warn( + logger.warn( `Running report-diffs-action is only supported for 'push', \ 'pull_request' and 'workflow_dispatch' events, but was triggered \ on a '${context.eventName}' event. Skipping execution.` @@ -97,19 +102,19 @@ export const runMeticulousTestsAction = async (): Promise => { }); if (shaToCompareAgainst != null && event.type === "pull_request") { - console.log( + logger.info( `Comparing screenshots for the commit head of this PR, ${shortSha( head )}, against ${shortSha(shaToCompareAgainst)}` ); } else if (shaToCompareAgainst != null) { - console.log( + logger.info( `Comparing screenshots for commit ${shortSha( head )} against commit ${shortSha(shaToCompareAgainst)}}` ); } else { - console.log(`Generating screenshots for commit ${shortSha(head)}`); + logger.info(`Generating screenshots for commit ${shortSha(head)}`); } const resultsReporter = new ResultsReporter({ @@ -218,11 +223,10 @@ const getOctokitOrFail = (githubToken: string | null) => { try { return getOctokit(githubToken); } catch (err) { - console.error(err); + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + logger.error(err); throw new Error( "Error connecting to GitHub. Did you specify a valid 'github-token'?" ); } }; - -const shortSha = (sha: string) => sha.slice(0, 7); diff --git a/src/utils/constants.ts b/src/utils/constants.ts index a6b5fd01..71d2b444 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -14,3 +14,5 @@ export const EXPECTED_PERMISSIONS_BLOCK = [ // the environment changes in a way that would cause a replay to behave differently, e.g. upgrading to a newer // replay-orchestrator-launcher version, or changing the version of puppeteer. export const LOGICAL_ENVIRONMENT_VERSION = 2; + +export const DOCS_URL = "https://app.meticulous.ai/docs/github-actions-v2"; diff --git a/src/utils/ensure-base-exists.utils.ts b/src/utils/ensure-base-exists.utils.ts index b324fffd..0f7301eb 100644 --- a/src/utils/ensure-base-exists.utils.ts +++ b/src/utils/ensure-base-exists.utils.ts @@ -9,7 +9,11 @@ import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import log from "loglevel"; import { Duration } from "luxon"; import { CodeChangeEvent } from "../types"; -import { LOGICAL_ENVIRONMENT_VERSION } from "./constants"; +import { DOCS_URL, LOGICAL_ENVIRONMENT_VERSION } from "./constants"; +import { + DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE, + isGithubPermissionsError, +} from "./error.utils"; import { getCurrentWorkflowId, getPendingWorkflowRun, @@ -68,7 +72,7 @@ export const ensureBaseTestsExists = async ({ }); if (testRun != null) { - logger.log(`Tests already exist for commit ${base} (${testRun.id})`); + logger.info(`Tests already exist for commit ${base} (${testRun.id})`); return { shaToCompareAgainst: base }; } @@ -82,7 +86,7 @@ export const ensureBaseTestsExists = async ({ octokit, }); if (alreadyPending != null) { - logger.log( + logger.info( `Waiting on workflow run on base commit (${base}) to compare against: ${alreadyPending.html_url}` ); @@ -163,7 +167,7 @@ export const ensureBaseTestsExists = async ({ return { shaToCompareAgainst: null }; } - logger.log(`Waiting on workflow run: ${workflowRun.html_url}`); + logger.info(`Waiting on workflow run: ${workflowRun.html_url}`); await waitForWorkflowCompletionAndThrowIfFailed({ owner, repo, @@ -251,11 +255,26 @@ const getHeadCommitForRef = async ({ ref: string; octokit: InstanceType; }): Promise => { - const result = await octokit.rest.repos.getBranch({ - owner, - repo, - branch: ref, - }); - const commitSha = result.data.commit.sha; - return commitSha; + try { + const result = await octokit.rest.repos.getBranch({ + owner, + repo, + branch: ref, + }); + const commitSha = result.data.commit.sha; + return commitSha; + } catch (err: unknown) { + if (isGithubPermissionsError(err)) { + // https://docs.github.com/en/rest/overview/permissions-required-for-github-apps?apiVersion=2022-11-28#repository-permissions-for-contents + throw new Error( + `Missing permission to get the head commit of the branch '${ref}'. This is required in order to correctly calculate the two commits to compare.` + + ` Please add the 'contents: read' permission to your workflow YAML file: see ${DOCS_URL} for the correct setup.` + ); + } + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + logger.error( + `Unable to get head commit of branch '${ref}'. This is required in order to correctly calculate the two commits to compare. ${DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE}` + ); + throw err; + } }; diff --git a/src/utils/error.utils.ts b/src/utils/error.utils.ts new file mode 100644 index 00000000..f6f8a047 --- /dev/null +++ b/src/utils/error.utils.ts @@ -0,0 +1,29 @@ +import { DOCS_URL } from "./constants"; + +export const isGithubPermissionsError = ( + error: MaybeOctokitRequestError | unknown +): boolean => { + const message = getErrorMessage(error); + return ( + message.toLowerCase().includes("resource not accessible by integration") || + (error as MaybeOctokitRequestError)?.status === 403 + ); +}; + +export const getErrorMessage = (error: unknown) => { + const message = (error as MaybeOctokitRequestError)?.message ?? ""; + return typeof message === "string" ? message : ""; +}; + +// Octokit returns RequestErrors, which have a message and a code +// But we can't make any strong assumptions, so we type it on the safe side +// https://github.com/octokit/request-error.js/blob/372097e9b16f70d4ad75089003dc9154e304faa7/src/index.ts#L16 +type MaybeOctokitRequestError = + | { + message?: string | unknown; + status?: number | unknown; + } + | null + | undefined; + +export const DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE = `Please check www.githubstatus.com, and that you have setup the action correctly, including with the correct permissions: see ${DOCS_URL} for the correct setup.`; diff --git a/src/utils/get-base-and-head-commit-shas.ts b/src/utils/get-base-and-head-commit-shas.ts index 0ff4e4b5..d0a4a912 100644 --- a/src/utils/get-base-and-head-commit-shas.ts +++ b/src/utils/get-base-and-head-commit-shas.ts @@ -1,5 +1,7 @@ import { execSync } from "child_process"; import { context } from "@actions/github"; +import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; +import log from "loglevel"; import { CodeChangeEvent } from "../types"; interface BaseAndHeadCommitShas { @@ -53,6 +55,8 @@ const tryGetMergeBaseOfHeadCommit = ( pullRequestBaseSha: string, baseRef: string ): string | null => { + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + try { markGitDirectoryAsSafe(); // Only a single commit is fetched by the checkout action by default @@ -69,7 +73,7 @@ const tryGetMergeBaseOfHeadCommit = ( if (!isValidGitSha(mergeBase)) { // Note: the GITHUB_SHA is always a merge commit, even if the merge is a no-op because the PR is up to date // So this should never happen - console.error( + logger.error( `Failed to get merge base of ${pullRequestHeadSha} and ${baseRef}: value returned by 'git merge-base' was not a valid git SHA ('${mergeBase}').` + `Using the base of the pull request instead (${pullRequestBaseSha}).` ); @@ -78,7 +82,7 @@ const tryGetMergeBaseOfHeadCommit = ( return mergeBase; } catch (error) { - console.error( + logger.error( `Failed to get merge base of ${pullRequestHeadSha} and ${baseRef}. Error: ${error}. Using the base of the pull request instead (${pullRequestBaseSha}).` ); return null; @@ -90,9 +94,10 @@ const tryGetMergeBaseOfTemporaryMergeCommit = ( pullRequestBaseSha: string ): string | null => { const mergeCommitSha = process.env.GITHUB_SHA; + const logger = log.getLogger(METICULOUS_LOGGER_NAME); if (mergeCommitSha == null) { - console.error( + logger.error( `No GITHUB_SHA environment var set, so can't work out true base of the merge commit. Using the base of the pull request instead (${pullRequestBaseSha}).` ); return null; @@ -104,7 +109,7 @@ const tryGetMergeBaseOfTemporaryMergeCommit = ( .toString() .trim(); if (headCommitSha !== mergeCommitSha) { - console.log( + logger.info( `The head commit SHA (${headCommitSha}) does not equal GITHUB_SHA environment variable (${mergeCommitSha}). This is likely because a custom ref has been passed to the 'actions/checkout' action. We're assuming therefore that the head commit SHA is not a temporary merge commit, but rather the head of the branch. Therefore we're @@ -124,7 +129,7 @@ const tryGetMergeBaseOfTemporaryMergeCommit = ( if (parents.length !== 2) { // Note: the GITHUB_SHA is always a merge commit, even if the merge is a no-op because the PR is up to date // So this should never happen - console.error( + logger.error( `GITHUB_SHA (${mergeCommitSha}) is not a merge commit, so can't work out true base of the merge commit. Using the base of the pull request instead.` ); return null; @@ -134,7 +139,7 @@ const tryGetMergeBaseOfTemporaryMergeCommit = ( const mergeBaseSha = parents[0]; const mergeHeadSha = parents[1]; if (mergeHeadSha !== pullRequestHeadSha) { - console.error( + logger.error( `The second parent (${parents[1]}) of the GITHUB_SHA merge commit (${mergeCommitSha}) is not equal to the head of the PR (${pullRequestHeadSha}), so can not confidently determine the base of the merge commit to compare against. Using the base of the pull request instead (${pullRequestBaseSha}).` ); @@ -142,7 +147,7 @@ const tryGetMergeBaseOfTemporaryMergeCommit = ( } return mergeBaseSha; } catch (e) { - console.error( + logger.error( `Error getting base of merge commit (${mergeCommitSha}). Using the base of the pull request instead (${pullRequestBaseSha}).`, e ); diff --git a/src/utils/logger.utils.ts b/src/utils/logger.utils.ts index c02b57ec..c1b76d28 100644 --- a/src/utils/logger.utils.ts +++ b/src/utils/logger.utils.ts @@ -33,3 +33,5 @@ export const setLogLevel: (logLevel: string | undefined) => void = ( break; } }; + +export const shortSha = (sha: string) => sha.slice(0, 7); diff --git a/src/utils/results-reporter.ts b/src/utils/results-reporter.ts index d68edf2c..90e914e7 100644 --- a/src/utils/results-reporter.ts +++ b/src/utils/results-reporter.ts @@ -1,10 +1,18 @@ import { getOctokit } from "@actions/github"; import { Project } from "@alwaysmeticulous/api"; +import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import { ExecuteTestRunResult, RunningTestRunExecution, } from "@alwaysmeticulous/sdk-bundles-api"; +import log from "loglevel"; import { CodeChangeEvent } from "../types"; +import { DOCS_URL } from "./constants"; +import { + DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE, + isGithubPermissionsError, +} from "./error.utils"; +import { shortSha } from "./logger.utils"; import { updateStatusComment } from "./update-status-comment"; const SHORT_SHA_LENGTH = 7; @@ -160,18 +168,35 @@ export class ResultsReporter { targetUrl?: string; }) { const { octokit, owner, repo, headSha } = this.options; - return octokit.rest.repos.createCommitStatus({ - owner, - repo, - context: - this.options.testSuiteId != null - ? `Meticulous (${this.options.testSuiteId})` - : "Meticulous", - description, - state, - sha: headSha, - ...(targetUrl ? { target_url: targetUrl } : {}), - }); + try { + return octokit.rest.repos.createCommitStatus({ + owner, + repo, + context: + this.options.testSuiteId != null + ? `Meticulous (${this.options.testSuiteId})` + : "Meticulous", + description, + state, + sha: headSha, + ...(targetUrl ? { target_url: targetUrl } : {}), + }); + } catch (err: unknown) { + if (isGithubPermissionsError(err)) { + // https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs + throw new Error( + `Missing permission to create and update commit statuses.` + + ` Please add the 'statuses: write' permission to your workflow YAML file: see ${DOCS_URL} for the correct setup.` + ); + } + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + logger.error( + `Unable to create commit status for commit '${shortSha( + headSha + )}'. ${DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE}` + ); + throw err; + } } private setStatusComment({ diff --git a/src/utils/update-status-comment.ts b/src/utils/update-status-comment.ts index 4b88f7e8..101bcbfe 100644 --- a/src/utils/update-status-comment.ts +++ b/src/utils/update-status-comment.ts @@ -1,5 +1,12 @@ import { getOctokit } from "@actions/github"; +import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; +import log from "loglevel"; import { CodeChangeEvent } from "../types"; +import { DOCS_URL } from "./constants"; +import { + DEFAULT_FAILED_OCTOKIT_REQUEST_MESSAGE, + isGithubPermissionsError, +} from "./error.utils"; const getMeticulousCommentIdentifier = (testSuiteId: string | null) => `