diff --git a/__tests__/summary.test.ts b/__tests__/summary.test.ts index e94813d..2bece84 100644 --- a/__tests__/summary.test.ts +++ b/__tests__/summary.test.ts @@ -219,7 +219,7 @@ test('addLicensesToSummary() - does not include entire section if no license iss expect(text).toEqual('') }) -test('addLicensesToSummary() - includes all license issues', () => { +test('addLicensesToSummary() - includes all license issues in table', () => { const licenseIssues = { forbidden: [createTestChange()], unresolved: [createTestChange(), createTestChange()], @@ -230,9 +230,27 @@ test('addLicensesToSummary() - includes all license issues', () => { const text = core.summary.stringify() expect(text).toContain('

License Issues

') - expect(text).toContain('

Incompatible Licenses

') - expect(text).toContain('

Unknown Licenses

') - expect(text).toContain('

Invalid SPDX License Definitions

') + expect(text).toContain('Incompatible License') + expect(text).toContain('Invalid SPDX License') + expect(text).toContain('Unknown License') +}) + +test('addLicenseToSummary() - adds one table per manifest', () => { + const licenseIssues = { + forbidden: [ + createTestChange({manifest: 'package.json'}), + createTestChange({manifest: '.github/workflows/test.yml'}) + ], + unresolved: [], + unlicensed: [] + } + + summary.addLicensesToSummary(licenseIssues, defaultConfig) + + const text = core.summary.stringify() + + expect(text).toContain('

package.json

') + expect(text).toContain('

.github/workflows/test.yml

') }) test('addLicensesToSummary() - does not include specific license type sub-section if nothing is found', () => { @@ -245,9 +263,9 @@ test('addLicensesToSummary() - does not include specific license type sub-sectio summary.addLicensesToSummary(licenseIssues, defaultConfig) const text = core.summary.stringify() - expect(text).not.toContain('

Incompatible Licenses

') - expect(text).not.toContain('

Unknown Licenses

') - expect(text).toContain('

Invalid SPDX License Definitions

') + expect(text).not.toContain('Incompatible License') + expect(text).not.toContain('Unknown License') + expect(text).toContain('Invalid SPDX License') }) test('addLicensesToSummary() - includes list of configured allowed licenses', () => { diff --git a/scripts/create_summary.ts b/scripts/create_summary.ts index 53588dc..92f4951 100644 --- a/scripts/create_summary.ts +++ b/scripts/create_summary.ts @@ -57,12 +57,24 @@ const createFullSummary = async (): Promise => { createTestChange({ name: 'octoinvader', license: 'Non SPDX License' + }), + createTestChange({ + name: 'owner/action-1', + license: 'XYZ-License', + version: 'v1.2.2', + manifest: '.github/workflows/action.yml' }) ], unlicensed: [ createTestChange({ name: 'my-other-dependency', - license: 'No License' + license: null + }), + createTestChange({ + name: 'owner/action-2', + version: 'main', + license: null, + manifest: '.github/workflows/action.yml' }) ] } diff --git a/src/licenses.ts b/src/licenses.ts index 76fc966..ccd26ad 100644 --- a/src/licenses.ts +++ b/src/licenses.ts @@ -14,7 +14,10 @@ import {isSPDXValid, octokitClient} from './utils' * @param { { allow?: string[], deny?: string[]}} licenses An object with `allow`/`deny` keys, each containing a list of licenses. * @returns {Promise<{Object.>}} A promise to a Record Object. The keys are strings, unlicensed, unresolved and forbidden. The values are a list of changes */ -type InvalidLicenseChangeTypes = 'unlicensed' | 'unresolved' | 'forbidden' +export type InvalidLicenseChangeTypes = + | 'unlicensed' + | 'unresolved' + | 'forbidden' export type InvalidLicenseChanges = Record export async function getInvalidLicenseChanges( changes: Change[], diff --git a/src/summary.ts b/src/summary.ts index 031da5d..8c868fc 100644 --- a/src/summary.ts +++ b/src/summary.ts @@ -1,7 +1,7 @@ import * as core from '@actions/core' import {ConfigurationOptions, Changes} from './schemas' import {SummaryTableRow} from '@actions/core/lib/summary' -import {InvalidLicenseChanges} from './licenses' +import {InvalidLicenseChanges, InvalidLicenseChangeTypes} from './licenses' import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils' const icons = { @@ -131,6 +131,7 @@ export function addLicensesToSummary( } core.summary.addHeading('License Issues', 2) + printLicenseViolations(invalidLicenseChanges) if (config.allow_licenses && config.allow_licenses.length > 0) { core.summary.addQuote( @@ -151,40 +152,44 @@ export function addLicensesToSummary( `${invalidLicenseChanges.unresolved.length} licenses could not be validated` ) - printLicenseViolation( - `Incompatible Licenses`, - invalidLicenseChanges.forbidden - ) - printLicenseViolation( - `Invalid SPDX License Definitions`, - invalidLicenseChanges.unresolved - ) - printLicenseViolation(`Unknown Licenses`, invalidLicenseChanges.unlicensed) core.summary.addSeparator() } -function printLicenseViolation(heading: string, changes: Changes): void { - if (changes.length === 0) { - return - } - core.summary.addSeparator() - core.summary.addHeading(heading, 3) +const licenseIssueTypes: InvalidLicenseChangeTypes[] = [ + 'forbidden', + 'unresolved', + 'unlicensed' +] - const rows: SummaryTableRow[] = [] - const manifests = getManifestsSet(changes) +const issueTypeNames: Record = { + forbidden: 'Incompatible License', + unresolved: 'Invalid SPDX License', + unlicensed: 'Unknown License' +} - for (const manifest of manifests) { - core.summary.addHeading(`${manifest}`, 4) +function printLicenseViolations(changes: InvalidLicenseChanges): void { + const rowsGroupedByManifest: Record = {} - for (const change of changes.filter(pkg => pkg.manifest === manifest)) { - rows.push([ + for (const issueType of licenseIssueTypes) { + for (const change of changes[issueType]) { + if (!rowsGroupedByManifest[change.manifest]) { + rowsGroupedByManifest[change.manifest] = [] + } + rowsGroupedByManifest[change.manifest].push([ renderUrl(change.source_repository_url, change.name), change.version, - formatLicense(change.license) + formatLicense(change.license), + issueTypeNames[issueType] ]) } + } - core.summary.addTable([['Package', 'Version', 'License'], ...rows]) + for (const [manifest, rows] of Object.entries(rowsGroupedByManifest)) { + core.summary.addHeading(`${manifest}`, 4) + core.summary.addTable([ + ['Package', 'Version', 'License', 'Issue Type'], + ...rows + ]) } }