Skip to content

Commit

Permalink
Do not list changes dependencies in summary
Browse files Browse the repository at this point in the history
  • Loading branch information
hmaurer authored and elireisman committed Sep 16, 2024
1 parent b3559aa commit 83c7cc6
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 76 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
20 changes: 6 additions & 14 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
22 changes: 6 additions & 16 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 Down Expand Up @@ -263,21 +263,11 @@ function formatLicense(license: string | null): string {
return license
}

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

const summary = core.summary.addHeading('Scanned Manifest Files', 2)

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>`)
}
}
export function addScannedFiles(changes: Changes): void {
const manifests = Array.from(
groupDependenciesByManifest(changes).keys()
).sort()
core.summary.addHeading('Scanned Files', 2).addList(manifests)
}

function snapshotWarningRecommendation(
Expand Down

0 comments on commit 83c7cc6

Please sign in to comment.