diff --git a/lib/init-action.js b/lib/init-action.js index f34a1bc2f..3ea0dfbf6 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -106378,25 +106378,21 @@ async function prepareDiffInformedAnalysis(codeql, features, logger) { logger.warning( `Failed to determine branch information for diff-informed analysis: ${getErrorMessage(e)}` ); - return { shouldRun: false, hasDiffRanges: false }; + return false; } if (!branches) { - return { shouldRun: false, hasDiffRanges: false }; + return false; } - const hasDiffRanges = await withGroupAsync( - "Computing PR diff ranges", - async () => { - try { - return await computeAndPersistDiffRanges(branches, logger); - } catch (e) { - logger.warning( - `Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}` - ); - return false; - } + return await withGroupAsync("Computing PR diff ranges", async () => { + try { + return await computeAndPersistDiffRanges(branches, logger); + } catch (e) { + logger.warning( + `Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}` + ); + return false; } - ); - return { shouldRun: true, hasDiffRanges }; + }); } function writeDiffRangesJsonFile(logger, ranges) { const jsonContents = JSON.stringify(ranges, null, 2); @@ -107283,14 +107279,20 @@ function userConfigFromActionPath(tempDir) { function hasQueryCustomisation(userConfig) { return isDefined2(userConfig["disable-default-queries"]) || isDefined2(userConfig.queries) || isDefined2(userConfig["query-filters"]); } -function applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger) { - if (config.overlayDatabaseMode === "overlay" /* Overlay */ && diffInformedAnalysis.shouldRun && !diffInformedAnalysis.hasDiffRanges) { +async function applyIncrementalAnalysisSettings(config, hasDiffRanges, codeql, logger) { + if (config.overlayDatabaseMode === "overlay" /* Overlay */ && !hasDiffRanges) { logger.info( - `Diff-informed analysis is not available for this pull request. Reverting overlay database mode to ${"none" /* None */}.` + `Reverting overlay database mode to ${"none" /* None */} because the PR diff ranges could not be computed.` ); config.overlayDatabaseMode = "none" /* None */; + config.useOverlayDatabaseCaching = false; + await addOverlayDisablementDiagnostics( + config, + codeql, + "pr-diff-ranges-not-computed" /* PrDiffRangesNotComputed */ + ); } - if (config.overlayDatabaseMode === "overlay" /* Overlay */ || diffInformedAnalysis.hasDiffRanges) { + if (hasDiffRanges) { config.extraQueryExclusions.push({ exclude: { tags: "exclude-from-incremental" } }); @@ -107405,12 +107407,17 @@ async function initConfig(features, inputs) { overlayDisabledReason ); } - const diffInformedAnalysis = await prepareDiffInformedAnalysis( + const hasDiffRanges = await prepareDiffInformedAnalysis( inputs.codeql, inputs.features, logger ); - applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger); + await applyIncrementalAnalysisSettings( + config, + hasDiffRanges, + inputs.codeql, + logger + ); if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) { const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime( inputs.codeql, diff --git a/src/config-utils.test.ts b/src/config-utils.test.ts index c75f4e225..417b61c75 100644 --- a/src/config-utils.test.ts +++ b/src/config-utils.test.ts @@ -2201,14 +2201,16 @@ test.serial( }, ); -test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff-informed is unavailable", (t) => { +test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff ranges are unavailable", async (t) => { const config = createTestConfig({}); config.overlayDatabaseMode = OverlayDatabaseMode.None; + const codeql = createStubCodeQL({}); const logger = getRunnerLogger(true); - configUtils.applyIncrementalAnalysisSettings( + await configUtils.applyIncrementalAnalysisSettings( config, - { shouldRun: false, hasDiffRanges: false }, + false, + codeql, logger, ); @@ -2216,14 +2218,17 @@ test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff- t.deepEqual(config.extraQueryExclusions, []); }); -test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions when diff-informed analysis shouldn't run", (t) => { - const config = createTestConfig({}); - config.overlayDatabaseMode = OverlayDatabaseMode.Overlay; +test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions when diff ranges are available", async (t) => { + const config = createTestConfig({ + overlayDatabaseMode: OverlayDatabaseMode.Overlay, + }); + const codeql = createStubCodeQL({}); const logger = getRunnerLogger(true); - configUtils.applyIncrementalAnalysisSettings( + await configUtils.applyIncrementalAnalysisSettings( config, - { shouldRun: false, hasDiffRanges: false }, + true, + codeql, logger, ); @@ -2233,30 +2238,36 @@ test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions w ]); }); -test("applyIncrementalAnalysisSettings: disables overlay analysis when diff-informed analysis is unavailable", (t) => { +test("applyIncrementalAnalysisSettings: disables overlay analysis when diff ranges are unavailable", async (t) => { const config = createTestConfig({ overlayDatabaseMode: OverlayDatabaseMode.Overlay, }); + config.useOverlayDatabaseCaching = true; + const codeql = createStubCodeQL({}); const logger = getRunnerLogger(true); - configUtils.applyIncrementalAnalysisSettings( + await configUtils.applyIncrementalAnalysisSettings( config, - { shouldRun: true, hasDiffRanges: false }, + false, + codeql, logger, ); t.is(config.overlayDatabaseMode, OverlayDatabaseMode.None); + t.is(config.useOverlayDatabaseCaching, false); t.deepEqual(config.extraQueryExclusions, []); }); -test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only runs", (t) => { +test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only runs", async (t) => { const config = createTestConfig({}); config.overlayDatabaseMode = OverlayDatabaseMode.None; + const codeql = createStubCodeQL({}); const logger = getRunnerLogger(true); - configUtils.applyIncrementalAnalysisSettings( + await configUtils.applyIncrementalAnalysisSettings( config, - { shouldRun: true, hasDiffRanges: true }, + true, + codeql, logger, ); diff --git a/src/config-utils.ts b/src/config-utils.ts index 0fd2d33b4..563cd31c4 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -31,10 +31,7 @@ import { addNoLanguageDiagnostic, makeTelemetryDiagnostic, } from "./diagnostics"; -import { - type DiffInformedAnalysisPreparation, - prepareDiffInformedAnalysis, -} from "./diff-informed-analysis-utils"; +import { prepareDiffInformedAnalysis } from "./diff-informed-analysis-utils"; import { EnvVar } from "./environment"; import * as errorMessages from "./error-messages"; import { Feature, FeatureEnablement } from "./feature-flags"; @@ -1082,39 +1079,39 @@ function hasQueryCustomisation(userConfig: UserConfig): boolean { /** * Finalize the incremental-analysis configuration for this run. * - * If overlay mode was selected for a PR but diff-informed analysis should have - * run and could not be prepared, fall back to a full non-overlay analysis. - * Query exclusions for incremental-only queries are applied only when the final - * configuration still uses overlay analysis or the diff ranges are available. + * Overlay analysis has only been validated in combination with diff-informed + * analysis, so if `Overlay` mode was selected for a pull request but the diff + * ranges could not be computed, fall back to a full non-overlay analysis. * - * Note that `overlayDatabaseMode === Overlay` does not imply - * `diffInformedAnalysis.shouldRun`. Overlay mode is selected based on language - * and feature-flag state and can apply outside of pull-request contexts (e.g. - * on branch pushes that build up the overlay cache), whereas diff-informed - * analysis only runs for pull requests where we can compute a diff. Each - * combination is therefore handled explicitly. + * Query exclusions for incremental-only queries are then applied whenever the + * diff ranges are available — which, after the fallback above, is exactly the + * set of runs where any kind of incremental analysis (overlay or + * diff-informed) is in effect. */ -export function applyIncrementalAnalysisSettings( +export async function applyIncrementalAnalysisSettings( config: Config, - diffInformedAnalysis: DiffInformedAnalysisPreparation, + hasDiffRanges: boolean, + codeql: CodeQL, logger: Logger, -): void { +): Promise { if ( config.overlayDatabaseMode === OverlayDatabaseMode.Overlay && - diffInformedAnalysis.shouldRun && - !diffInformedAnalysis.hasDiffRanges + !hasDiffRanges ) { logger.info( - "Diff-informed analysis is not available for this pull request. " + - `Reverting overlay database mode to ${OverlayDatabaseMode.None}.`, + `Reverting overlay database mode to ${OverlayDatabaseMode.None} ` + + "because the PR diff ranges could not be computed.", ); config.overlayDatabaseMode = OverlayDatabaseMode.None; + config.useOverlayDatabaseCaching = false; + await addOverlayDisablementDiagnostics( + config, + codeql, + OverlayDisabledReason.PrDiffRangesNotComputed, + ); } - if ( - config.overlayDatabaseMode === OverlayDatabaseMode.Overlay || - diffInformedAnalysis.hasDiffRanges - ) { + if (hasDiffRanges) { config.extraQueryExclusions.push({ exclude: { tags: "exclude-from-incremental" }, }); @@ -1275,13 +1272,18 @@ export async function initConfig( ); } - const diffInformedAnalysis = await prepareDiffInformedAnalysis( + const hasDiffRanges = await prepareDiffInformedAnalysis( inputs.codeql, inputs.features, logger, ); - applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger); + await applyIncrementalAnalysisSettings( + config, + hasDiffRanges, + inputs.codeql, + logger, + ); if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) { const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime( diff --git a/src/diff-informed-analysis-utils.test.ts b/src/diff-informed-analysis-utils.test.ts index b1c0b14b7..96fce23eb 100644 --- a/src/diff-informed-analysis-utils.test.ts +++ b/src/diff-informed-analysis-utils.test.ts @@ -190,7 +190,7 @@ test.serial( ); test.serial( - "prepareDiffInformedAnalysis: returns shouldRun=false when not a pull request", + "prepareDiffInformedAnalysis: returns false when not a pull request", async (t) => { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); @@ -209,13 +209,13 @@ test.serial( logger, ); - t.deepEqual(result, { shouldRun: false, hasDiffRanges: false }); + t.false(result); }); }, ); test.serial( - "prepareDiffInformedAnalysis: returns shouldRun=false when applicability check throws", + "prepareDiffInformedAnalysis: returns false when applicability check throws", async (t) => { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); @@ -239,13 +239,13 @@ test.serial( logger, ); - t.deepEqual(result, { shouldRun: false, hasDiffRanges: false }); + t.false(result); }); }, ); test.serial( - "prepareDiffInformedAnalysis: returns hasDiffRanges=true when the diff is fetched successfully", + "prepareDiffInformedAnalysis: returns true when the diff is fetched successfully", async (t) => { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); @@ -276,13 +276,13 @@ test.serial( logger, ); - t.deepEqual(result, { shouldRun: true, hasDiffRanges: true }); + t.true(result); }); }, ); test.serial( - "prepareDiffInformedAnalysis: returns hasDiffRanges=false when the diff API call fails", + "prepareDiffInformedAnalysis: returns false when the diff API call fails", async (t) => { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); @@ -313,7 +313,7 @@ test.serial( logger, ); - t.deepEqual(result, { shouldRun: true, hasDiffRanges: false }); + t.false(result); }); }, ); diff --git a/src/diff-informed-analysis-utils.ts b/src/diff-informed-analysis-utils.ts index 9760d873d..a48c6dcfd 100644 --- a/src/diff-informed-analysis-utils.ts +++ b/src/diff-informed-analysis-utils.ts @@ -55,30 +55,18 @@ export async function getDiffInformedAnalysisBranches( return branches; } -export interface DiffInformedAnalysisPreparation { - /** - * Whether diff-informed analysis applies to this workflow run. - */ - shouldRun: boolean; - /** - * Whether the diff ranges were successfully computed and persisted, and are - * therefore available for use. - */ - hasDiffRanges: boolean; -} - /** * Prepares the diff ranges needed for diff-informed analysis for the current * run. * - * @returns Whether diff-informed analysis applies to this run, and whether the - * diff ranges were successfully computed and persisted. + * @returns `true` if the diff ranges were successfully computed and persisted + * and are therefore available for use, `false` otherwise. */ export async function prepareDiffInformedAnalysis( codeql: CodeQL, features: FeatureEnablement, logger: Logger, -): Promise { +): Promise { let branches: PullRequestBranches | undefined; try { branches = await getDiffInformedAnalysisBranches(codeql, features, logger); @@ -89,26 +77,22 @@ export async function prepareDiffInformedAnalysis( logger.warning( `Failed to determine branch information for diff-informed analysis: ${getErrorMessage(e)}`, ); - return { shouldRun: false, hasDiffRanges: false }; + return false; } if (!branches) { - return { shouldRun: false, hasDiffRanges: false }; + return false; } - const hasDiffRanges = await withGroupAsync( - "Computing PR diff ranges", - async () => { - try { - return await computeAndPersistDiffRanges(branches, logger); - } catch (e) { - logger.warning( - `Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`, - ); - return false; - } - }, - ); - return { shouldRun: true, hasDiffRanges }; + return await withGroupAsync("Computing PR diff ranges", async () => { + try { + return await computeAndPersistDiffRanges(branches, logger); + } catch (e) { + logger.warning( + `Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`, + ); + return false; + } + }); } export interface DiffThunkRange { diff --git a/src/overlay/diagnostics.ts b/src/overlay/diagnostics.ts index 6bc11a73f..d350e38f3 100644 --- a/src/overlay/diagnostics.ts +++ b/src/overlay/diagnostics.ts @@ -39,6 +39,13 @@ export enum OverlayDisabledReason { NotPullRequestOrDefaultBranch = "not-pull-request-or-default-branch", /** The top-level overlay analysis feature flag is not enabled. */ OverallFeatureNotEnabled = "overall-feature-not-enabled", + /** + * Overlay analysis was selected for a pull request, but the PR diff ranges + * needed for diff-informed analysis could not be computed. Overlay analysis + * has only been validated in combination with diff-informed analysis, so we + * fall back to a non-overlay analysis in this case. + */ + PrDiffRangesNotComputed = "pr-diff-ranges-not-computed", /** Overlay analysis was skipped because it previously failed with similar hardware resources. */ SkippedDueToCachedStatus = "skipped-due-to-cached-status", /** Disk usage could not be determined during the overlay status check. */