Skip to content

Commit

Permalink
Merge pull request #828 from actions/hm/summary
Browse files Browse the repository at this point in the history
Do not list changed dependencies in summary
  • Loading branch information
elireisman authored Sep 16, 2024
2 parents b3559aa + 05042db commit 6ea3b24
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 70 deletions.
46 changes: 3 additions & 43 deletions __tests__/summary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,42 +109,6 @@ test('prints headline as h1', () => {
expect(text).toContain('<h1>Dependency Review</h1>')
})

test('returns minimal summary in case the core.summary is too large for a PR comment', () => {
let changes: Changes = [
createTestChange({name: 'lodash', version: '1.2.3'}),
createTestChange({name: 'colors', version: '2.3.4'}),
createTestChange({name: '@foo/bar', version: '*'})
]

let minSummary: string = summary.addSummaryToSummary(
changes,
emptyInvalidLicenseChanges,
emptyChanges,
scorecard,
defaultConfig
)

// side effect DR report into core.summary as happens in main.ts
summary.addScannedDependencies(changes)
const text = core.summary.stringify()

expect(text).toContain('<h1>Dependency Review</h1>')
expect(minSummary).toContain('# Dependency Review')

expect(text).toContain('❌ 3 vulnerable package(s)')
expect(text).not.toContain('* ❌ 3 vulnerable package(s)')
expect(text).toContain('lodash')
expect(text).toContain('colors')
expect(text).toContain('@foo/bar')

expect(minSummary).toContain('* ❌ 3 vulnerable package(s)')
expect(minSummary).not.toContain('lodash')
expect(minSummary).not.toContain('colors')
expect(minSummary).not.toContain('@foo/bar')

expect(text.length).toBeGreaterThan(minSummary.length)
})

test('returns minimal summary formatted for posting as a PR comment', () => {
const OLD_ENV = process.env

Expand Down Expand Up @@ -232,14 +196,10 @@ test('groups dependencies with empty manifest paths together', () => {
emptyScorecard,
defaultConfig
)
summary.addScannedDependencies(changesWithEmptyManifests)
summary.addScannedFiles(changesWithEmptyManifests)
const text = core.summary.stringify()

expect(text).toContain('<summary>Unnamed Manifest</summary>')
expect(text).toContain('castore')
expect(text).toContain('connection')
expect(text).toContain('<summary>python/dist-info/METADATA</summary>')
expect(text).toContain('pygments')
expect(text).toContain('Unnamed Manifest')
expect(text).toContain('python/dist-info/METADATA')
})

test('does not include status section if nothing was found', () => {
Expand Down
35 changes: 23 additions & 12 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion scripts/create_summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ async function createSummary(
...licenseIssues.unlicensed
]

summary.addScannedDependencies(allChanges)
summary.addScannedFiles(allChanges)

const text = core.summary.stringify()
await fs.promises.writeFile(path.resolve(tmpDir, fileName), text, {
Expand Down
2 changes: 1 addition & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ async function run(): Promise<void> {
}

core.setOutput('dependency-changes', JSON.stringify(changes))
summary.addScannedDependencies(changes)
summary.addScannedFiles(changes)
printScannedDependencies(changes)

// include full summary in output; Actions will truncate if oversized
Expand Down
38 changes: 26 additions & 12 deletions src/summary.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as core from '@actions/core'
import {ConfigurationOptions, Changes, Change, Scorecard} from './schemas'
import {SummaryTableRow} from '@actions/core/lib/summary'
import {InvalidLicenseChanges, InvalidLicenseChangeTypes} from './licenses'
import {Change, Changes, ConfigurationOptions, Scorecard} from './schemas'
import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils'

const icons = {
Expand All @@ -10,6 +10,8 @@ const icons = {
warning: '⚠️'
}

const MAX_SCANNED_FILES_BYTES = 1048576

// generates the DR report summmary and caches it to the Action's core.summary.
// returns the DR summary string, ready to be posted as a PR comment if the
// final DR report is too large
Expand Down Expand Up @@ -263,21 +265,33 @@ function formatLicense(license: string | null): string {
return license
}

export function addScannedDependencies(changes: Changes): void {
const dependencies = groupDependenciesByManifest(changes)
const manifests = dependencies.keys()
export function addScannedFiles(changes: Changes): void {
const manifests = Array.from(
groupDependenciesByManifest(changes).keys()
).sort()

const summary = core.summary.addHeading('Scanned Manifest Files', 2)
let sf_size = 0
let trunc_at = -1

for (const manifest of manifests) {
const deps = dependencies.get(manifest)
if (deps) {
const dependencyNames = deps.map(
dependency => `<li>${dependency.name}@${dependency.version}</li>`
)
summary.addDetails(manifest, `<ul>${dependencyNames.join('')}</ul>`)
for (const [index, entry] of manifests.entries()) {
if (sf_size + entry.length >= MAX_SCANNED_FILES_BYTES) {
trunc_at = index
break
}
sf_size += entry.length
}

if (trunc_at >= 0) {
// truncate the manifests list if it will overflow the summary output
manifests.slice(0, trunc_at)
// if there's room between cutoff size and list size, add a warning
const size_diff = MAX_SCANNED_FILES_BYTES - sf_size
if (size_diff < 12) {
manifests.push('(truncated)')
}
}

core.summary.addHeading('Scanned Files', 2).addList(manifests)
}

function snapshotWarningRecommendation(
Expand Down

0 comments on commit 6ea3b24

Please sign in to comment.