Skip to content

Commit da529b3

Browse files
committed
Return cache keys with exact hits from downloadDependencyCaches
1 parent 85f1517 commit da529b3

File tree

4 files changed

+79
-29
lines changed

4 files changed

+79
-29
lines changed

lib/init-action.js

Lines changed: 17 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/dependency-caching.test.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,17 @@ test("downloadDependencyCaches - does not restore caches with feature keys if no
237237
.resolves(CSHARP_BASE_PATTERNS);
238238
makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined);
239239

240-
const results = await downloadDependencyCaches(
240+
const result = await downloadDependencyCaches(
241241
codeql,
242242
createFeatures([]),
243243
[KnownLanguage.csharp],
244244
logger,
245245
);
246-
t.is(results.length, 1);
247-
t.is(results[0].language, KnownLanguage.csharp);
248-
t.is(results[0].hit_kind, CacheHitKind.Miss);
246+
const statusReport = result.statusReport;
247+
t.is(statusReport.length, 1);
248+
t.is(statusReport[0].language, KnownLanguage.csharp);
249+
t.is(statusReport[0].hit_kind, CacheHitKind.Miss);
250+
t.deepEqual(result.exactKeys, []);
249251
t.assert(restoreCacheStub.calledOnce);
250252
});
251253

@@ -257,7 +259,8 @@ test("downloadDependencyCaches - restores caches with feature keys if features a
257259
const logger = getRecordingLogger(messages);
258260
const features = createFeatures([Feature.CsharpNewCacheKey]);
259261

260-
sinon.stub(glob, "hashFiles").resolves("abcdef");
262+
const mockHash = "abcdef";
263+
sinon.stub(glob, "hashFiles").resolves(mockHash);
261264

262265
const keyWithFeature = await cacheKey(
263266
codeql,
@@ -277,15 +280,28 @@ test("downloadDependencyCaches - restores caches with feature keys if features a
277280
.resolves(CSHARP_BASE_PATTERNS);
278281
makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined);
279282

280-
const results = await downloadDependencyCaches(
283+
const result = await downloadDependencyCaches(
281284
codeql,
282285
features,
283286
[KnownLanguage.csharp],
284287
logger,
285288
);
286-
t.is(results.length, 1);
287-
t.is(results[0].language, KnownLanguage.csharp);
288-
t.is(results[0].hit_kind, CacheHitKind.Exact);
289+
290+
// Check that the status report for telemetry indicates that one cache was restored with an exact match.
291+
const statusReport = result.statusReport;
292+
t.is(statusReport.length, 1);
293+
t.is(statusReport[0].language, KnownLanguage.csharp);
294+
t.is(statusReport[0].hit_kind, CacheHitKind.Exact);
295+
296+
// Check that the exact key match has been returned.
297+
const exactKeys = result.exactKeys;
298+
t.is(exactKeys.length, 1);
299+
t.assert(
300+
exactKeys[0].endsWith(mockHash),
301+
"Expected exact key match to end with hash returned by `hashFiles`",
302+
);
303+
304+
// `restoreCache` should have been called exactly once.
289305
t.assert(restoreCacheStub.calledOnce);
290306
});
291307

@@ -319,15 +335,17 @@ test("downloadDependencyCaches - restores caches with feature keys if features a
319335
.resolves(CSHARP_BASE_PATTERNS);
320336
makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined);
321337

322-
const results = await downloadDependencyCaches(
338+
const result = await downloadDependencyCaches(
323339
codeql,
324340
features,
325341
[KnownLanguage.csharp],
326342
logger,
327343
);
328-
t.is(results.length, 1);
329-
t.is(results[0].language, KnownLanguage.csharp);
330-
t.is(results[0].hit_kind, CacheHitKind.Partial);
344+
const statusReport = result.statusReport;
345+
t.is(statusReport.length, 1);
346+
t.is(statusReport[0].language, KnownLanguage.csharp);
347+
t.is(statusReport[0].hit_kind, CacheHitKind.Partial);
348+
t.deepEqual(result.exactKeys, []);
331349
t.assert(restoreCacheStub.calledOnce);
332350
});
333351

src/dependency-caching.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,14 @@ export interface DependencyCacheRestoreStatus {
193193
/** An array of `DependencyCacheRestoreStatus` objects for each analysed language with a caching configuration. */
194194
export type DependencyCacheRestoreStatusReport = DependencyCacheRestoreStatus[];
195195

196+
/* Represents the results of `downloadDependencyCaches`. */
197+
export interface DownloadDependencyCachesResult {
198+
/* The status report for telemetry */
199+
statusReport: DependencyCacheRestoreStatusReport;
200+
/* An array of cache keys that we had exact cache hits for. */
201+
exactKeys: string[];
202+
}
203+
196204
/**
197205
* A wrapper around `cacheConfig.getHashPatterns` which logs when there are no files to calculate
198206
* a hash for the cache key from.
@@ -239,8 +247,9 @@ export async function downloadDependencyCaches(
239247
features: FeatureEnablement,
240248
languages: Language[],
241249
logger: Logger,
242-
): Promise<DependencyCacheRestoreStatusReport> {
250+
): Promise<DownloadDependencyCachesResult> {
243251
const status: DependencyCacheRestoreStatusReport = [];
252+
const exactKeys: string[] = [];
244253

245254
for (const language of languages) {
246255
const cacheConfig = defaultCacheConfigs[language];
@@ -288,16 +297,28 @@ export async function downloadDependencyCaches(
288297

289298
if (hitKey !== undefined) {
290299
logger.info(`Cache hit on key ${hitKey} for ${language}.`);
291-
const hit_kind =
292-
hitKey === primaryKey ? CacheHitKind.Exact : CacheHitKind.Partial;
293-
status.push({ language, hit_kind, download_duration_ms });
300+
301+
// We have a partial cache hit, unless the key of the restored cache matches the
302+
// primary restore key. In that case we have an exact hit and track the restored
303+
// cache key so that we can skip storing the same cache again later.
304+
let hit_kind = CacheHitKind.Partial;
305+
if (hitKey === primaryKey) {
306+
hit_kind = CacheHitKind.Exact;
307+
exactKeys.push(hitKey);
308+
}
309+
310+
status.push({
311+
language,
312+
hit_kind,
313+
download_duration_ms,
314+
});
294315
} else {
295316
status.push({ language, hit_kind: CacheHitKind.Miss });
296317
logger.info(`No suitable cache found for ${language}.`);
297318
}
298319
}
299320

300-
return status;
321+
return { statusReport: status, exactKeys };
301322
}
302323

303324
/** Enumerates possible outcomes for storing caches. */

src/init-action.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ async function run() {
371371
}
372372

373373
let overlayBaseDatabaseStats: OverlayBaseDatabaseDownloadStats | undefined;
374-
let dependencyCachingResults: DependencyCacheRestoreStatusReport | undefined;
374+
let dependencyCachingStatus: DependencyCacheRestoreStatusReport | undefined;
375375
try {
376376
if (
377377
config.overlayDatabaseMode === OverlayDatabaseMode.Overlay &&
@@ -579,12 +579,13 @@ async function run() {
579579

580580
// Restore dependency cache(s), if they exist.
581581
if (shouldRestoreCache(config.dependencyCachingEnabled)) {
582-
dependencyCachingResults = await downloadDependencyCaches(
582+
const dependencyCachingResult = await downloadDependencyCaches(
583583
codeql,
584584
features,
585585
config.languages,
586586
logger,
587587
);
588+
dependencyCachingStatus = dependencyCachingResult.statusReport;
588589
}
589590

590591
// Suppress warnings about disabled Python library extraction.
@@ -732,7 +733,7 @@ async function run() {
732733
toolsSource,
733734
toolsVersion,
734735
overlayBaseDatabaseStats,
735-
dependencyCachingResults,
736+
dependencyCachingStatus,
736737
logger,
737738
error,
738739
);
@@ -755,7 +756,7 @@ async function run() {
755756
toolsSource,
756757
toolsVersion,
757758
overlayBaseDatabaseStats,
758-
dependencyCachingResults,
759+
dependencyCachingStatus,
759760
logger,
760761
);
761762
}

0 commit comments

Comments
 (0)