8000 Merge pull request #828 from actions/hm/summary · actions/dependency-review-action@6ea3b24 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6ea3b24

Browse files
elireismanelireisman
authored
Merge pull request #828 from actions/hm/summary
Do not list changed dependencies in summary
2 parents b3559aa + 05042db commit 6ea3b24

File tree

6 files changed

+55
-70
lines changed

6 files changed

+55
-70
lines changed

‎__tests__/summary.test.ts

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -109,42 +109,6 @@ test('prints headline as h1', () => {
109109
expect(text).toContain('<h1>Dependency Review</h1>')
110110
})
111111

112-
test('returns minimal summary in case the core.summary is too large for a PR comment', () => {
113-
let changes: Changes = [
114-
createTestChange({name: 'lodash', version: '1.2.3'}),
115-
createTestChange({name: 'colors', version: '2.3.4'}),
116-
createTestChange({name: '@foo/bar', version: '*'})
117-
]
118-
119-
let minSummary: string = summary.addSummaryToSummary(
120-
changes,
121-
emptyInvalidLicenseChanges,
122-
emptyChanges,
123-
scorecard,
124-
defaultConfig
125-
)
126-
127-
// side effect DR report into core.summary as happens in main.ts
128-
summary.addScannedDependencies(changes)
129-
const text = core.summary.stringify()
130-
131-
expect(text).toContain('<h1>Dependency Review</h1>')
132-
expect(minSummary).toContain('# Dependency Review')
133-
134-
expect(text).toContain('❌ 3 vulnerable package(s)')
135-
expect(text).not.toContain('* ❌ 3 vulnerable package(s)')
136-
expect(text).toContain('lodash')
137-
expect(text).toContain('colors')
138-
expect(text).toContain('@foo/bar')
139-
140-
expect(minSummary).toContain('* ❌ 3 vulnerable package(s)')
141-
expect(minSummary).not.toContain('lodash')
142-
expect(minSummary).not.toContain('colors')
143-
expect(minSummary).not.toContain('@foo/bar')
144-
145-
expect(text.length).toBeGreaterThan(minSummary.length)
146-
})
147-
148112
test('returns minimal summary formatted for posting as a PR comment', () => {
149113
const OLD_ENV = process.env
150114

@@ -232,14 +196,10 @@ test('groups dependencies with empty manifest paths together', () => {
232196
emptyScorecard,
233197
defaultConfig
234198
)
235-
summary.addScannedDependencies(changesWithEmptyManifests)
199+
summary.addScannedFiles(changesWithEmptyManifests)
236200
const text = core.summary.stringify()
237-
238-
expect(text).toContain('<summary>Unnamed Manifest</summary>')
239-
expect(text).toContain('castore')
240-
expect(text).toContain('connection')
241-
expect(text).toContain('<summary>python/dist-info/METADATA</summary>')
242-
expect(text).toContain('pygments')
201+
expect(text).toContain('Unnamed Manifest')
202+
expect(text).toContain('python/dist-info/METADATA')
243203
})
244204

245205
test('does not include status section if nothing was found', () => {

‎dist/index.js

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

‎dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎scripts/create_summary.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ async function createSummary(
143143
...licenseIssues.unlicensed
144144
]
145145

146-
summary.addScannedDependencies(allChanges)
146+
summary.addScannedFiles(allChanges)
147147

148148
const text = core.summary.stringify()
149149
await fs.promises.writeFile(path.resolve(tmpDir, fileName), text, {

‎src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ async function run(): Promise<void> {
166166
}
167167

168168
core.setOutput('dependency-changes', JSON.stringify(changes))
169-
summary.addScannedDependencies(changes)
169+
summary.addScannedFiles(changes)
170170
printScannedDependencies(changes)
171171

172172
// include full summary in output; Actions will truncate if oversized

‎src/summary.ts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as core from '@actions/core'
2-
import {ConfigurationOptions, Changes, Change, Scorecard} from './schemas'
32
import {SummaryTableRow} from '@actions/core/lib/summary'
43
import {InvalidLicenseChanges, InvalidLicenseChangeTypes} from './licenses'
4+
import {Change, Changes, ConfigurationOptions, Scorecard} from './schemas'
55
import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils'
66

77
const icons = {
@@ -10,6 +10,8 @@ const icons = {
1010
warning: '⚠️'
1111
}
1212

13+
const MAX_SCANNED_FILES_BYTES = 1048576
14+
1315
// generates the DR report summmary and caches it to the Action's core.summary.
1416
// returns the DR summary string, ready to be posted as a PR comment if the
1517
// final DR report is too large
@@ -263,21 +265,33 @@ function formatLicense(license: string | null): string {
263265
return license
264266
}
265267

266-
export function addScannedDependencies(changes: Changes): void {
267-
const dependencies = groupDependenciesByManifest(changes)
268-
const manifests = dependencies.keys()
268+
export function addScannedFiles(changes: Changes): void {
269+
const manifests = Array.from(
270+
groupDependenciesByManifest(changes).keys()
271+
).sort()
269272

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

272-
for (const manifest of manifests) {
273-
const deps = dependencies.get(manifest)
274-
if (deps) {
275-
const dependencyNames = deps.map(
276-
dependency => `<li>${dependency.name}@${dependency.version}</li>`
277-
)
278-
summary.addDetails(manifest, `<ul>${dependencyNames.join('')}</ul>`)
276+
for (const [index, entry] of manifests.entries()) {
277+
if (sf_size + entry.length >= MAX_SCANNED_FILES_BYTES) {
278+
trunc_at = index
279+
break
279280
}
281+
sf_size += entry.length
280282
}
283+
284+
if (trunc_at >= 0) {
285+
// truncate the manifests list if it will overflow the summary output
286+
manifests.slice(0, trunc_at)
287+
// if there's room between cutoff size and list size, add a warning
288+
const size_diff = MAX_SCANNED_FILES_BYTES - sf_size
289+
if (size_diff < 12) {
290+
manifests.push('(truncated)')
291+
}
292+
}
293+
294+
core.summary.addHeading('Scanned Files', 2).addList(manifests)
281295
}
282296

283297
function snapshotWarningRecommendation(

0 commit comments

Comments
 (0)
0