mirror of
https://github.com/gradle/actions.git
synced 2026-07-28 15:14:31 +02:00
Redesign the caching Job Summary (#985)
Redesigns the caching section of the Job Summary into a single, consistent layout across every cache provider and state, and integrates the provider message into the report rather than appending it disconnected at the bottom. ## Motivation The caching report was produced by three divergent code paths (NoOp / basic / enhanced), each rendering its own markdown: - **Explicitly disabled** → a one-line message, no expand, no provider note. - **Enhanced** (incl. skipped-due-to-existing-home) → a full `<details>` block. - **Basic** → a one-line message with **no** expandable details at all. The Enhanced/Basic provider note floated at the very bottom, disconnected from the report. ## What changed `save()` now returns structured `CacheReport` data instead of pre-rendered HTML, and a single renderer (`caching-report.ts`) produces one unified layout for all variants: - **Section heading**: `#### <icon> Gradle Caching — <Provider> (<status>)` - **Status line** explaining what the cache did - **Integrated provider note** woven in under the heading — now shown **unconditionally** (no longer gated on license acceptance) - **Expandable cache-entry details** when there are entries — basic caching now gets this too The two disabled variants (explicitly disabled, and skipped due to a pre-existing Gradle User Home) render as **compact callouts with no expandable section**. ### Main repo - `caching-report.ts` (new): central renderer + all framing copy + entry table/`<pre>` helpers. - `cache-service.ts`: `CacheReport` / `CacheEntryReport` / status types; `save()` returns `CacheReport`. - `cache-service-loader.ts`: `NoOp` returns a report; `LicenseWarningCacheService` removed; new `getProviderNote()`. - `cache-service-basic.ts`: builds a `CacheReport`. - `job-summary.ts` / `setup-gradle.ts`: thread `CacheReport` + `ProviderNote`. - `configuration.ts`: remove now-unused `isCacheLicenseAccepted()`. ### Vendored library The structured contract requires **gradle-actions-caching v0.7.0** (gradle/actions-caching#74). This PR updates the vendored library to that release — the official `Update gradle-actions-caching library to v0.7.0` vendor commit is included here, so merging this PR ships the redesign together with the library it depends on. ## Testing - Both repos build; prettier + eslint clean. - `gradle/actions`: 363/363 Jest tests pass, including new `caching-report.test.ts` covering every variant. - `gradle-actions-caching`: 74/74 pass under JDK 17. - Rendered markdown verified for all five variants (enhanced/basic enabled & read-only, disabled, skipped). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Bot Githubaction <bot-githubaction@gradle.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Bot Githubaction
Claude Opus 4.8
parent
8b6cdb5f58
commit
97715a29bc
@@ -138,8 +138,8 @@ describe('BasicCacheService', () => {
|
||||
})
|
||||
|
||||
expect(mockSaveCache).not.toHaveBeenCalled()
|
||||
expect(report).toContain('read-only')
|
||||
expect(report).toContain(PRIMARY_KEY)
|
||||
expect(report.status).toBe('read-only')
|
||||
expect(report.entries[0].restoredKey).toBe(PRIMARY_KEY)
|
||||
})
|
||||
|
||||
it('reports readOnly with no restore when cache was missed', async () => {
|
||||
@@ -156,8 +156,9 @@ describe('BasicCacheService', () => {
|
||||
})
|
||||
|
||||
expect(mockSaveCache).not.toHaveBeenCalled()
|
||||
expect(report).toContain('read-only')
|
||||
expect(report).toContain('No cache entry')
|
||||
expect(report.status).toBe('read-only')
|
||||
expect(report.entries[0].restoredKey).toBeUndefined()
|
||||
expect(report.entries[0].restoredOutcome).toContain('not restored')
|
||||
})
|
||||
|
||||
it('skips save when restored key equals primary key', async () => {
|
||||
@@ -179,7 +180,8 @@ describe('BasicCacheService', () => {
|
||||
})
|
||||
|
||||
expect(mockSaveCache).not.toHaveBeenCalled()
|
||||
expect(report).toContain('Save was skipped')
|
||||
expect(report.status).toBe('enabled')
|
||||
expect(report.entries[0].savedOutcome).toContain('already exists')
|
||||
})
|
||||
|
||||
it('saves cache and returns report on success', async () => {
|
||||
@@ -204,7 +206,9 @@ describe('BasicCacheService', () => {
|
||||
['/home/.gradle/caches', '/home/.gradle/wrapper'],
|
||||
PRIMARY_KEY
|
||||
)
|
||||
expect(report).toContain('saved entry with key')
|
||||
expect(report.status).toBe('enabled')
|
||||
expect(report.entries[0].savedKey).toBe(PRIMARY_KEY)
|
||||
expect(report.entries[0].savedOutcome).toBe('(Entry saved)')
|
||||
})
|
||||
|
||||
it('warns on save failure instead of throwing', async () => {
|
||||
@@ -228,7 +232,7 @@ describe('BasicCacheService', () => {
|
||||
expect(mockWarning).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed to save')
|
||||
)
|
||||
expect(report).toContain('failed')
|
||||
expect(report.entries[0].savedOutcome).toContain('not saved')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,8 +12,7 @@ describe('getCacheService selection logic', () => {
|
||||
const {getCacheService} = await import('../../src/cache-service-loader')
|
||||
const mockConfig = {
|
||||
isCacheDisabled: () => true,
|
||||
getCacheProvider: () => CacheProvider.Enhanced,
|
||||
isCacheLicenseAccepted: () => true
|
||||
getCacheProvider: () => CacheProvider.Enhanced
|
||||
} as unknown as CacheConfig
|
||||
|
||||
const service = await getCacheService(mockConfig)
|
||||
@@ -28,23 +27,53 @@ describe('getCacheService selection logic', () => {
|
||||
excludes: []
|
||||
})
|
||||
|
||||
// NoOpCacheService returns a specific report mentioning cache was disabled
|
||||
expect(report).toContain('Cache was disabled')
|
||||
// NoOpCacheService reports a disabled cache with no entries
|
||||
expect(report.status).toBe('disabled')
|
||||
expect(report.entries).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('wraps BasicCacheService with LicenseWarningCacheService when cache-provider is basic', async () => {
|
||||
it('returns a BasicCacheService when cache-provider is basic', async () => {
|
||||
const {getCacheService} = await import('../../src/cache-service-loader')
|
||||
const mockConfig = {
|
||||
isCacheDisabled: () => false,
|
||||
getCacheProvider: () => CacheProvider.Basic,
|
||||
isCacheLicenseAccepted: () => false
|
||||
getCacheProvider: () => CacheProvider.Basic
|
||||
} as unknown as CacheConfig
|
||||
|
||||
const service = await getCacheService(mockConfig)
|
||||
|
||||
// The service should not be a bare BasicCacheService — it should be wrapped
|
||||
// with LicenseWarningCacheService that appends the basic caching summary
|
||||
const {BasicCacheService} = await import('../../src/cache-service-basic')
|
||||
expect(service).not.toBeInstanceOf(BasicCacheService)
|
||||
expect(service).toBeInstanceOf(BasicCacheService)
|
||||
})
|
||||
|
||||
describe('getProviderNote', () => {
|
||||
it('returns undefined when cache is disabled', async () => {
|
||||
const {getProviderNote} = await import('../../src/cache-service-loader')
|
||||
const mockConfig = {
|
||||
isCacheDisabled: () => true,
|
||||
getCacheProvider: () => CacheProvider.Enhanced
|
||||
} as unknown as CacheConfig
|
||||
|
||||
expect(getProviderNote(mockConfig)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns basic note for the basic provider', async () => {
|
||||
const {getProviderNote} = await import('../../src/cache-service-loader')
|
||||
const mockConfig = {
|
||||
isCacheDisabled: () => false,
|
||||
getCacheProvider: () => CacheProvider.Basic
|
||||
} as unknown as CacheConfig
|
||||
|
||||
expect(getProviderNote(mockConfig)).toEqual({kind: 'basic'})
|
||||
})
|
||||
|
||||
it('returns enhanced note for the enhanced provider', async () => {
|
||||
const {getProviderNote} = await import('../../src/cache-service-loader')
|
||||
const mockConfig = {
|
||||
isCacheDisabled: () => false,
|
||||
getCacheProvider: () => CacheProvider.Enhanced
|
||||
} as unknown as CacheConfig
|
||||
|
||||
expect(getProviderNote(mockConfig)).toEqual({kind: 'enhanced'})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import {describe, expect, it} from '@jest/globals'
|
||||
|
||||
import {CacheReport} from '../../src/cache-service'
|
||||
import {renderCachingReport} from '../../src/caching-report'
|
||||
|
||||
const ENHANCED = {kind: 'enhanced'} as const
|
||||
const BASIC = {kind: 'basic'} as const
|
||||
|
||||
function entry(overrides: Partial<CacheReport['entries'][number]> = {}): CacheReport['entries'][number] {
|
||||
return {
|
||||
entryName: 'Gradle User Home',
|
||||
requestedKey: 'gradle-home-v1|key',
|
||||
restoredKey: 'gradle-home-v1|key',
|
||||
restoredSize: 535792,
|
||||
restoredTime: 253,
|
||||
restoredOutcome: '(Entry restored: exact match found)',
|
||||
savedKey: 'gradle-home-v1|key',
|
||||
savedSize: 528509,
|
||||
savedTime: 257,
|
||||
savedOutcome: '(Entry saved)',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('renderCachingReport', () => {
|
||||
it('renders an enhanced read-only report with heading, note and details', () => {
|
||||
const report: CacheReport = {status: 'read-only', cleanup: 'disabled-readonly', entries: [entry()]}
|
||||
const md = renderCachingReport(report, ENHANCED)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - ⚡ Enhanced (read-only)</h4>')
|
||||
expect(md).toContain('[Enhanced Caching]')
|
||||
expect(md).toContain('`gradle-actions-caching`')
|
||||
expect(md).toContain('DISTRIBUTION.md')
|
||||
expect(md).toContain('<details>')
|
||||
expect(md).toContain('<summary>Entries: 1 restored (1Mb), 1 saved (1Mb) - Expand for more details</summary>')
|
||||
// status message moves inside the details expando
|
||||
expect(md).toContain('Cache was read-only')
|
||||
// read-only does not render the cleanup line
|
||||
expect(md).not.toContain('Cache cleanup')
|
||||
})
|
||||
|
||||
it('renders an enhanced enabled report with status and cleanup inside the details', () => {
|
||||
const report: CacheReport = {status: 'enabled', cleanup: 'enabled', entries: [entry()]}
|
||||
const md = renderCachingReport(report, ENHANCED)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - ⚡ Enhanced</h4>')
|
||||
// status and cleanup messages are within the expando, after the summary
|
||||
const detailsBody = md.slice(md.indexOf('</summary>'))
|
||||
expect(detailsBody).toContain('Cache was enabled')
|
||||
expect(detailsBody).toContain('Cache cleanup')
|
||||
expect(md).toContain('<table>')
|
||||
expect(md).toContain('Entries Restored')
|
||||
})
|
||||
|
||||
it('renders a basic report with the upgrade note and no metrics table', () => {
|
||||
const report: CacheReport = {
|
||||
status: 'read-only',
|
||||
entries: [
|
||||
entry({
|
||||
restoredSize: undefined,
|
||||
restoredTime: undefined,
|
||||
savedKey: undefined,
|
||||
savedSize: undefined,
|
||||
savedTime: undefined,
|
||||
savedOutcome: '(Entry not saved: cache is read-only)'
|
||||
})
|
||||
]
|
||||
}
|
||||
const md = renderCachingReport(report, BASIC)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - 🛡️ Basic (read-only)</h4>')
|
||||
expect(md).toContain('[Basic Caching]')
|
||||
expect(md).toContain('[Enhanced Caching]')
|
||||
// DISTRIBUTION.md is only referenced for the enhanced provider
|
||||
expect(md).not.toContain('DISTRIBUTION.md')
|
||||
// No size/time data, so the metrics table is omitted but the entry list remains
|
||||
expect(md).not.toContain('<table>')
|
||||
expect(md).toContain('<pre>')
|
||||
expect(md).toContain('<summary>Entries: 1 restored, 0 saved - Expand for more details</summary>')
|
||||
})
|
||||
|
||||
it('renders a compact disabled report with no note and no details', () => {
|
||||
const report: CacheReport = {status: 'disabled', entries: []}
|
||||
const md = renderCachingReport(report, undefined)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - Disabled</h4>')
|
||||
expect(md).toContain('Caching was disabled')
|
||||
expect(md).not.toContain('<details>')
|
||||
expect(md).not.toContain('DISTRIBUTION.md')
|
||||
})
|
||||
|
||||
it('renders a compact skipped report for a pre-existing Gradle User Home', () => {
|
||||
const report: CacheReport = {status: 'disabled-existing-home', entries: []}
|
||||
const md = renderCachingReport(report, ENHANCED)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - Skipped</h4>')
|
||||
expect(md).toContain('pre-existing Gradle User Home')
|
||||
expect(md).not.toContain('<details>')
|
||||
// no provider note for non-active states
|
||||
expect(md).not.toContain('DISTRIBUTION.md')
|
||||
})
|
||||
|
||||
it('renders an unavailable report compactly', () => {
|
||||
const report: CacheReport = {status: 'not-available', entries: []}
|
||||
const md = renderCachingReport(report, ENHANCED)
|
||||
|
||||
expect(md).toContain('<h4>Gradle State Caching - Unavailable</h4>')
|
||||
expect(md).not.toContain('<details>')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user