Skip to content

Commit 11889c2

Browse files
committed
Return keys of restored caches from downloadDependencyCaches
1 parent 85f1517 commit 11889c2

File tree

4 files changed

+95
-30
lines changed

4 files changed

+95
-30
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: 48 additions & 14 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.restoredKeys, []);
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 restored key has been returned.
297+
const restoredKeys = result.restoredKeys;
298+
t.is(restoredKeys.length, 1);
299+
t.assert(
300+
restoredKeys[0].endsWith(mockHash),
301+
"Expected restored key to end with hash returned by `hashFiles`",
302+
);
303+
304+
// `restoreCache` should have been called exactly once.
289305
t.assert(restoreCacheStub.calledOnce);
290306
});
291307

@@ -297,8 +313,14 @@ test("downloadDependencyCaches - restores caches with feature keys if features a
297313
const logger = getRecordingLogger(messages);
298314
const features = createFeatures([Feature.CsharpNewCacheKey]);
299315

316+
// We expect two calls to `hashFiles`: the first by the call to `cacheKey` below,
317+
// and the second by `downloadDependencyCaches`. We use the result of the first
318+
// call as part of the cache key that identifies a mock, existing cache. The result
319+
// of the second call is for the primary restore key, which we don't want to match
320+
// the first key so that we can test the restore keys logic.
321+
const restoredHash = "abcdef";
300322
const hashFilesStub = sinon.stub(glob, "hashFiles");
301-
hashFilesStub.onFirstCall().resolves("abcdef");
323+
hashFilesStub.onFirstCall().resolves(restoredHash);
302324
hashFilesStub.onSecondCall().resolves("123456");
303325

304326
const keyWithFeature = await cacheKey(
@@ -319,15 +341,27 @@ test("downloadDependencyCaches - restores caches with feature keys if features a
319341
.resolves(CSHARP_BASE_PATTERNS);
320342
makePatternCheckStub.withArgs(CSHARP_EXTRA_PATTERNS).resolves(undefined);
321343

322-
const results = await downloadDependencyCaches(
344+
const result = await downloadDependencyCaches(
323345
codeql,
324346
features,
325347
[KnownLanguage.csharp],
326348
logger,
327349
);
328-
t.is(results.length, 1);
329-
t.is(results[0].language, KnownLanguage.csharp);
330-
t.is(results[0].hit_kind, CacheHitKind.Partial);
350+
351+
// Check that the status report for telemetry indicates that one cache was restored with a partial match.
352+
const statusReport = result.statusReport;
353+
t.is(statusReport.length, 1);
354+
t.is(statusReport[0].language, KnownLanguage.csharp);
355+
t.is(statusReport[0].hit_kind, CacheHitKind.Partial);
356+
357+
// Check that the restored key has been returned.
358+
const restoredKeys = result.restoredKeys;
359+
t.is(restoredKeys.length, 1);
360+
t.assert(
361+
restoredKeys[0].endsWith(restoredHash),
362+
"Expected restored key to end with hash returned by `hashFiles`",
363+
);
364+
331365
t.assert(restoreCacheStub.calledOnce);
332366
});
333367

src/dependency-caching.ts

Lines changed: 25 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 have restored and therefore know to exist. */
201+
restoredKeys: 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 restoredKeys: string[] = [];
244253

245254
for (const language of languages) {
246255
const cacheConfig = defaultCacheConfigs[language];
@@ -288,16 +297,27 @@ 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.
303+
let hit_kind = CacheHitKind.Partial;
304+
if (hitKey === primaryKey) {
305+
hit_kind = CacheHitKind.Exact;
306+
}
307+
308+
status.push({
309+
language,
310+
hit_kind,
311+
download_duration_ms,
312+
});
313+
restoredKeys.push(hitKey);
294314
} else {
295315
status.push({ language, hit_kind: CacheHitKind.Miss });
296316
logger.info(`No suitable cache found for ${language}.`);
297317
}
298318
}
299319

300-
return status;
320+
return { statusReport: status, restoredKeys };
301321
}
302322

303323
/** 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)