Merge pull request #4245 from github/henrymercer/graduate-overlay-trap-caching

Graduate overlay TRAP caching defaults

Author: henrymercer

init/action.yml

@@ -153,7 +153,10 @@ inputs:
     required: false
   trap-caching:
     description: >-
-      Explicitly enable or disable TRAP caching rather than respecting the feature flag for it.
+      Explicitly enable or disable TRAP caching instead of using the automatically determined default.
+      TRAP caching is disabled by default when improved incremental analysis is enabled,
+      because improved incremental analysis of a pull request only extracts changed files,
+      which rarely benefit from TRAP caching.
     required: false
   dependency-caching:
     description: >-

lib/entry-points.js

@@ -2126,85 +2126,68 @@ test.serial("getPrimaryAnalysisConfig - Code Scanning + Code Quality", (t) => {
 });
 
 test.serial(
-  "isTrapCachingEnabled: explicit input true is respected",
+  "isTrapCachingEnabled: explicit true input overrides self-hosted default",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
       setupActionsVars(tmpDir, tmpDir);
       sinon
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
         .returns("true");
-      t.true(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([]),
-          OverlayDatabaseMode.None,
-        ),
-      );
+      t.true(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None));
     });
   },
 );
 
 test.serial(
-  "isTrapCachingEnabled: disabled on self-hosted runner by default",
+  "isTrapCachingEnabled: explicit true input overrides overlay defaults",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
+      setupActionsVars(tmpDir, path.join(tmpDir, "hostedtoolcache"));
       sinon
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
-        .returns(undefined);
-      t.false(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([]),
-          OverlayDatabaseMode.None,
-        ),
-      );
+        .returns("true");
+      for (const overlayDatabaseMode of [
+        OverlayDatabaseMode.Overlay,
+        OverlayDatabaseMode.OverlayBase,
+      ]) {
+        t.true(configUtils.isTrapCachingEnabled(overlayDatabaseMode));
+      }
     });
   },
 );
 
 test.serial(
-  "isTrapCachingEnabled: enabled on hosted runner by default",
+  "isTrapCachingEnabled: explicit false input overrides hosted default",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
-      const hostedToolCache = path.join(tmpDir, "hostedtoolcache");
-      setupActionsVars(tmpDir, hostedToolCache);
+      setupActionsVars(tmpDir, path.join(tmpDir, "hostedtoolcache"));
       sinon
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
-        .returns(undefined);
-      t.true(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([]),
-          OverlayDatabaseMode.None,
-        ),
-      );
+        .returns("false");
+      t.false(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None));
     });
   },
 );
 
 test.serial(
-  "isTrapCachingEnabled: enabled on hosted runner when overlay enabled but feature flag off",
+  "isTrapCachingEnabled: disabled on self-hosted runner by default",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
-      const hostedToolCache = path.join(tmpDir, "hostedtoolcache");
-      setupActionsVars(tmpDir, hostedToolCache);
+      setupActionsVars(tmpDir, tmpDir);
       sinon
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
         .returns(undefined);
-      t.true(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([]),
-          OverlayDatabaseMode.Overlay,
-        ),
-      );
+      t.false(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None));
     });
   },
 );
 
 test.serial(
-  "isTrapCachingEnabled: disabled on hosted runner when overlay enabled and feature flag on",
+  "isTrapCachingEnabled: enabled on hosted runner without overlay analysis",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
       const hostedToolCache = path.join(tmpDir, "hostedtoolcache");
@@ -2213,18 +2196,13 @@ test.serial(
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
         .returns(undefined);
-      t.false(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([Feature.OverlayAnalysisDisableTrapCaching]),
-          OverlayDatabaseMode.Overlay,
-        ),
-      );
+      t.true(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None));
     });
   },
 );
 
 test.serial(
-  "isTrapCachingEnabled: enabled on hosted runner when overlay is None even with feature flag on",
+  "isTrapCachingEnabled: disabled on hosted runner with overlay analysis",
   async (t) => {
     return await withTmpDir(async (tmpDir) => {
       const hostedToolCache = path.join(tmpDir, "hostedtoolcache");
@@ -2233,12 +2211,12 @@ test.serial(
         .stub(actionsUtil, "getOptionalInput")
         .withArgs("trap-caching")
         .returns(undefined);
-      t.true(
-        await configUtils.isTrapCachingEnabled(
-          createFeatures([Feature.OverlayAnalysisDisableTrapCaching]),
-          OverlayDatabaseMode.None,
-        ),
-      );
+      for (const overlayDatabaseMode of [
+        OverlayDatabaseMode.Overlay,
+        OverlayDatabaseMode.OverlayBase,
+      ]) {
+        t.false(configUtils.isTrapCachingEnabled(overlayDatabaseMode));
+      }
     });
   },
 );

src/config-utils.test.ts

@@ -944,23 +944,23 @@ async function validateOverlayDatabaseMode(
   });
 }
 
-export async function isTrapCachingEnabled(
-  features: FeatureEnablement,
+/**
+ * Determines whether TRAP caching is enabled for the current run.
+ *
+ * @returns Whether TRAP caches should be downloaded and uploaded.
+ */
+export function isTrapCachingEnabled(
   overlayDatabaseMode: OverlayDatabaseMode,
-): Promise<boolean> {
+): boolean {
   // If the workflow specified something, always respect that.
   const trapCaching = getOptionalInput("trap-caching");
   if (trapCaching !== undefined) return trapCaching === "true";
 
   // On self-hosted runners which may have slow network access, disable TRAP caching by default.
   if (!looksLikeHostedRunner()) return false;
 
-  // If overlay analysis is enabled, then disable TRAP caching since overlay analysis supersedes it.
-  // This change is gated behind a feature flag.
-  if (
-    overlayDatabaseMode !== OverlayDatabaseMode.None &&
-    (await features.getValue(Feature.OverlayAnalysisDisableTrapCaching))
-  ) {
+  // Overlay analysis supersedes TRAP caching, so disable TRAP caching by default when it is active.
+  if (overlayDatabaseMode !== OverlayDatabaseMode.None) {
     return false;
   }
 
@@ -1302,7 +1302,7 @@ export async function initConfig(
     logger,
   );
 
-  if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) {
+  if (isTrapCachingEnabled(config.overlayDatabaseMode)) {
     const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
       inputs.codeql,
       config.languages,

src/config-utils.ts

@@ -103,8 +103,6 @@ export enum Feature {
   OverlayAnalysisCodeScanningRuby = "overlay_analysis_code_scanning_ruby",
   OverlayAnalysisCpp = "overlay_analysis_cpp",
   OverlayAnalysisCsharp = "overlay_analysis_csharp",
-  /** Disable TRAP caching when overlay analysis is enabled. */
-  OverlayAnalysisDisableTrapCaching = "overlay_analysis_disable_trap_caching",
   OverlayAnalysisGo = "overlay_analysis_go",
   OverlayAnalysisJava = "overlay_analysis_java",
   OverlayAnalysisJavascript = "overlay_analysis_javascript",
@@ -369,11 +367,6 @@ export const featureConfig = {
     minimumVersion: CODEQL_OVERLAY_MINIMUM_VERSION_RUBY,
   },
   // Other overlay-related feature flags
-  [Feature.OverlayAnalysisDisableTrapCaching]: {
-    defaultValue: false,
-    envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_DISABLE_TRAP_CACHING",
-    minimumVersion: undefined,
-  },
   [Feature.OverlayAnalysisMatchCodeqlVersion]: {
     defaultValue: false,
     envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_MATCH_CODEQL_VERSION",