From 89f0a768fcc8e417de31655bd1ca7bfafd334bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Chalk?= Date: Fri, 9 Feb 2024 16:09:52 +0100 Subject: [PATCH] refactor(examples-plugins): remove source links in issues --- .../src/file-size/src/file-size.plugin.ts | 13 +--- .../src/file-size.plugin.unit.test.ts | 55 ++-------------- examples/plugins/src/lighthouse/src/utils.ts | 3 - .../src/integration/dependencies.audit.ts | 23 +------ .../dependencies.audit.unit.test.ts | 66 +++++-------------- .../src/integration/license.audit.ts | 3 - .../integration/license.audit.unit.test.ts | 19 +----- .../src/integration/type.audit.ts | 18 +---- .../src/integration/type.audit.unit.test.ts | 36 +--------- .../src/package-json/src/integration/utils.ts | 32 +-------- .../package-json.plugin.integration.test.ts | 15 ----- 11 files changed, 35 insertions(+), 248 deletions(-) diff --git a/examples/plugins/src/file-size/src/file-size.plugin.ts b/examples/plugins/src/file-size/src/file-size.plugin.ts index 571882479..1a8df2618 100644 --- a/examples/plugins/src/file-size/src/file-size.plugin.ts +++ b/examples/plugins/src/file-size/src/file-size.plugin.ts @@ -12,7 +12,6 @@ import { factorOf, formatBytes, pluralizeToken, - toUnixPath, } from '@code-pushup/utils'; export type PluginOptions = { @@ -165,12 +164,6 @@ export function assertFileSize( ): Issue { // ensure size positive numbers const formattedSize = Math.max(size, 0); - // informative issue - const issue = { - source: { - file: toUnixPath(file, { toRelative: true }), - }, - } satisfies Pick; if (budget !== undefined) { // ensure budget is positive numbers @@ -178,19 +171,17 @@ export function assertFileSize( // return error Issue if (budget < formattedSize) { return { - ...issue, severity: 'error', message: errorMessage(file, formattedSize, formattedBudget), - } satisfies Issue; + }; } } // return informative Issue return { - ...issue, severity: 'info', message: infoMessage(file, formattedSize), - } satisfies Issue; + }; } export default create; diff --git a/examples/plugins/src/file-size/src/file-size.plugin.unit.test.ts b/examples/plugins/src/file-size/src/file-size.plugin.unit.test.ts index e44057e4c..c610b7b64 100644 --- a/examples/plugins/src/file-size/src/file-size.plugin.unit.test.ts +++ b/examples/plugins/src/file-size/src/file-size.plugin.unit.test.ts @@ -60,7 +60,6 @@ describe('assertFileSize', () => { expect(assertFileSize('test.js', size)).toEqual({ message: infoMessage('test.js', size), severity: 'info', - source: { file: 'test.js' }, }); }, ); @@ -75,7 +74,6 @@ describe('assertFileSize', () => { expect(assertFileSize('test.js', size, budget)).toEqual({ message: infoMessage('test.js', size), severity: 'info', - source: { file: 'test.js' }, }); }, ); @@ -86,7 +84,6 @@ describe('assertFileSize', () => { expect(assertFileSize('test.js', size, budget)).toEqual({ message: errorMessage('test.js', size, budget), severity: 'error', - source: { file: 'test.js' }, }); }, ); @@ -110,15 +107,10 @@ describe('fileSizeIssues', () => { it('should list all files', async () => { await expect(fileSizeIssues(baseOptions)).resolves.toEqual( - expect.arrayContaining( - ['project.json', 'test.js', 'README.md'].map(f => ({ - message: expect.any(String), - severity: expect.any(String), - source: { - file: expect.stringContaining(f), - }, - })), - ), + Array.from({ length: 3 }).map(() => ({ + message: expect.any(String), + severity: expect.any(String), + })), ); }); @@ -132,9 +124,6 @@ describe('fileSizeIssues', () => { { message: expect.any(String), severity: expect.any(String), - source: { - file: expect.stringContaining('test.js'), - }, }, ]); }); @@ -150,23 +139,14 @@ describe('fileSizeIssues', () => { { message: expect.any(String), severity: 'info', - source: { - file: expect.stringContaining('README.md'), - }, }, { message: expect.any(String), severity: 'error', - source: { - file: expect.stringContaining('project.json'), - }, }, { message: expect.any(String), severity: 'error', - source: { - file: expect.stringContaining('test.js'), - }, }, ]), ); @@ -183,9 +163,6 @@ describe('fileSizeIssues', () => { { message: expect.any(String), severity: 'error', - source: { - file: expect.stringContaining('test.js'), - }, }, ]); }); @@ -248,27 +225,6 @@ describe('runnerFunction', () => { ]); }); - it('should have files in issues that are matching the pattern as issues', async () => { - await expect( - runnerFunction({ - ...baseOptions, - pattern: /\.js$/, - }), - ).resolves.toEqual([ - expect.objectContaining({ - details: { - issues: [ - expect.objectContaining({ - source: { - file: expect.stringContaining('test.js'), - }, - }), - ], - }, - }), - ]); - }); - it('should have number of files that are over budget as value and listed in issues', async () => { await expect( runnerFunction({ @@ -287,9 +243,6 @@ describe('runnerFunction', () => { message: 'File test.js has 154 B, this is 26 B too big. (budget: 128 B)', severity: 'error', - source: { - file: expect.stringContaining('test.js'), - }, }, ]), }, diff --git a/examples/plugins/src/lighthouse/src/utils.ts b/examples/plugins/src/lighthouse/src/utils.ts index 02d78c85e..4b127dfb5 100644 --- a/examples/plugins/src/lighthouse/src/utils.ts +++ b/examples/plugins/src/lighthouse/src/utils.ts @@ -78,9 +78,6 @@ export function lhrDetailsToIssueDetails( .join(',') .slice(0, MAX_ISSUE_MESSAGE_LENGTH), severity: 'info', - source: { - file: 'required-in-portal-api', - }, }, ]; } diff --git a/examples/plugins/src/package-json/src/integration/dependencies.audit.ts b/examples/plugins/src/package-json/src/integration/dependencies.audit.ts index 389152c85..eb4adf3ab 100644 --- a/examples/plugins/src/package-json/src/integration/dependencies.audit.ts +++ b/examples/plugins/src/package-json/src/integration/dependencies.audit.ts @@ -1,5 +1,5 @@ import { Audit, AuditOutput, Issue } from '@code-pushup/models'; -import { factorOf, findLineNumberInText } from '@code-pushup/utils'; +import { factorOf } from '@code-pushup/utils'; import { DependencyMap, DependencyType, @@ -74,7 +74,6 @@ export function dependenciesIssues( // Generate the appropriate issue based on whether the dependency exists return existingVersion === undefined ? packageNotInstalledIssue( - packageResult, [dependencyName, requiredVersion], dependencyType as DependencyType, ) @@ -91,19 +90,14 @@ export function dependenciesIssues( } export function packageNotInstalledIssue( - packageResult: Pick, requiredDependency: [string, string], dependencyType: DependencyType, ): Issue { - const { file } = packageResult; const [packageName, targetVersion] = requiredDependency; return { message: `Package ${packageName} is not installed under ${dependencyType}. Run \`npm install ${packageName}@${targetVersion}\` to install it.`, severity: 'error', - source: { - file, - }, - } satisfies Issue; + }; } export function assertDependency( @@ -111,30 +105,19 @@ export function assertDependency( requiredDependency: [string, string], dependencyType: DependencyType, ): Issue { - const { file = '', json = {}, content = '' } = packageResult; + const { json = {} } = packageResult; const [packageName, targetVersion] = requiredDependency; - const source: Issue['source'] = { - file, - }; - const existingVersion = json[dependencyType]?.[packageName]; if (targetVersion !== existingVersion) { return { severity: 'error', message: `Package ${packageName} in ${dependencyType} has wrong version. Wanted ${targetVersion} but got ${existingVersion}`, - source: { - ...source, - position: { - startLine: findLineNumberInText(content, `"${packageName}":`) ?? 0, - }, - }, }; } return { message: `Package ${packageName}@${targetVersion} is installed as ${dependencyType}.`, severity: 'info', - source, }; } diff --git a/examples/plugins/src/package-json/src/integration/dependencies.audit.unit.test.ts b/examples/plugins/src/package-json/src/integration/dependencies.audit.unit.test.ts index ccb1f9929..992b9c3be 100644 --- a/examples/plugins/src/package-json/src/integration/dependencies.audit.unit.test.ts +++ b/examples/plugins/src/package-json/src/integration/dependencies.audit.unit.test.ts @@ -1,10 +1,6 @@ import { describe, expect, it } from 'vitest'; import { AuditOutput } from '@code-pushup/models'; -import { - packageJson, - packageJsonName, - packageResult, -} from '../../../../mocks/constants'; +import { packageJson, packageResult } from '../../../../mocks/constants'; import { assertDependency, dependenciesAudit, @@ -13,24 +9,20 @@ import { describe('packageNotInstalledIssue', () => { it.each([ - [packageJsonName, 'lib1', '*'], - [packageJsonName, 'lib1', '^.0.0.0'], - [packageJsonName, 'lib1', '0.0.0'], - ])('should return correct issue', (file, packageName, targetVersion) => { - expect( - packageNotInstalledIssue( - { file }, - [packageName, targetVersion], - 'dependencies', - ), - ).toEqual({ - message: `Package ${packageName} is not installed under dependencies. Run \`npm install ${packageName}@${targetVersion}\` to install it.`, - severity: 'error', - source: { - file, - }, - }); - }); + ['lib1', '*'], + ['lib1', '^.0.0.0'], + ['lib1', '0.0.0'], + ])( + 'should return correct issue for package %p with version %p', + (packageName, targetVersion) => { + expect( + packageNotInstalledIssue([packageName, targetVersion], 'dependencies'), + ).toEqual({ + message: `Package ${packageName} is not installed under dependencies. Run \`npm install ${packageName}@${targetVersion}\` to install it.`, + severity: 'error', + }); + }, + ); }); describe('assertPackageVersion', () => { @@ -45,9 +37,6 @@ describe('assertPackageVersion', () => { ).toEqual({ message: `Package ${packageName}@0.0.0 is installed as dependencies.`, severity: 'info', - source: { - file: packageJsonName, - }, }); }); @@ -62,12 +51,6 @@ describe('assertPackageVersion', () => { ).toEqual({ message: `Package ${packageName} in dependencies has wrong version. Wanted 0.0.1 but got 0.0.0`, severity: 'error', - source: { - file: packageJsonName, - position: { - startLine: 1, - }, - }, }); }); }); @@ -106,9 +89,6 @@ describe('dependenciesAudit', () => { { message: `Package ${packageName}@${targetVersion} is installed as dependencies.`, severity: 'info', - source: { - file: 'package.json', - }, }, ], }, @@ -148,24 +128,19 @@ describe('dependenciesAudit', () => { issues: [ { message: expect.stringMatching(/^(?=.*foo)(?=.*dependencies).*/), - severity: 'info', - source: expect.any(Object), }, { message: expect.stringMatching( /^(?=.*bar)(?=.*devDependencies).*/, ), - severity: 'info', - source: expect.any(Object), }, { message: expect.stringMatching( /^(?=.*baz)(?=.*optionalDependencies).*/, ), severity: 'info', - source: expect.any(Object), }, ], }, @@ -194,12 +169,6 @@ describe('dependenciesAudit', () => { message: 'Package lib1 in dependencies has wrong version. Wanted 0.0.1 but got 0.0.0', severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, @@ -246,24 +215,19 @@ describe('dependenciesAudit', () => { issues: [ { message: expect.stringMatching(/^(?=.*foo)(?=.*dependencies).*/), - severity: 'error', - source: expect.any(Object), }, { message: expect.stringMatching( /^(?=.*bar)(?=.*devDependencies).*/, ), - severity: 'error', - source: expect.any(Object), }, { message: expect.stringMatching( /^(?=.*baz)(?=.*optionalDependencies).*/, ), severity: 'error', - source: expect.any(Object), }, ], }, diff --git a/examples/plugins/src/package-json/src/integration/license.audit.ts b/examples/plugins/src/package-json/src/integration/license.audit.ts index 77ffd6ef1..4f2a1512c 100644 --- a/examples/plugins/src/package-json/src/integration/license.audit.ts +++ b/examples/plugins/src/package-json/src/integration/license.audit.ts @@ -39,9 +39,6 @@ export function licenseAudit( return { message: `License is ${json.license}`, severity: 'info', - source: { - file, - }, }; }); diff --git a/examples/plugins/src/package-json/src/integration/license.audit.unit.test.ts b/examples/plugins/src/package-json/src/integration/license.audit.unit.test.ts index f7270d6cb..d3e00926a 100644 --- a/examples/plugins/src/package-json/src/integration/license.audit.unit.test.ts +++ b/examples/plugins/src/package-json/src/integration/license.audit.unit.test.ts @@ -33,11 +33,8 @@ describe('licenseAudit', () => { details: { issues: [ { - message: `license should be ANY-LICENSE but is undefined`, + message: 'license should be ANY-LICENSE but is undefined', severity: 'error', - source: { - file: 'package.json', - }, }, ], }, @@ -56,14 +53,8 @@ describe('licenseAudit', () => { details: { issues: [ { - message: `license should be ANY-LICENSE but is `, + message: 'license should be ANY-LICENSE but is ', severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, @@ -84,12 +75,6 @@ describe('licenseAudit', () => { { message: `license should be MIT but is ${targetPackageJson.license}`, severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, diff --git a/examples/plugins/src/package-json/src/integration/type.audit.ts b/examples/plugins/src/package-json/src/integration/type.audit.ts index 7d3175889..601c6c336 100644 --- a/examples/plugins/src/package-json/src/integration/type.audit.ts +++ b/examples/plugins/src/package-json/src/integration/type.audit.ts @@ -1,5 +1,4 @@ import { AuditOutput, Issue } from '@code-pushup/models'; -import { findLineNumberInText } from '@code-pushup/utils'; import { PackageJson, SourceResult, SourceResults } from './types'; import { assertPropertyEmpty, @@ -28,17 +27,14 @@ export function typeAudit( }; } - const issues: Issue[] = packageJsonContents.map(({ file, json, content }) => { + const issues = packageJsonContents.map(({ file, json, content }): Issue => { // If type is undefined is defaults to 'commonjs' // https://nodejs.org/docs/latest-v13.x/api/esm.html#esm_package_json_type_field if (type === 'commonjs') { if (json.type === undefined) { return { - message: `Type is undefined. Defaults to commonjs.`, + message: 'Type is undefined. Defaults to commonjs.', severity: 'info', - source: { - file, - }, }; } else if (type !== json.type) { return assertTypeNotCommonJS({ file, json, content }, type); @@ -56,9 +52,6 @@ export function typeAudit( return { message: `Type is ${json.type}`, severity: 'info', - source: { - file, - }, }; }); @@ -78,14 +71,9 @@ export function assertTypeNotCommonJS( result: SourceResult, value: unknown, ): Issue { - const { file, content, json } = result; - const startLine: null | number = findLineNumberInText(content, `"type":`); + const { json } = result; return { severity: 'error', message: `type should be undefined or ${value?.toString()} but is ${json.type?.toString()}`, - source: { - file, - ...(startLine == null ? {} : { position: { startLine } }), - }, }; } diff --git a/examples/plugins/src/package-json/src/integration/type.audit.unit.test.ts b/examples/plugins/src/package-json/src/integration/type.audit.unit.test.ts index 3457bd139..0897951f9 100644 --- a/examples/plugins/src/package-json/src/integration/type.audit.unit.test.ts +++ b/examples/plugins/src/package-json/src/integration/type.audit.unit.test.ts @@ -42,9 +42,6 @@ describe('typeAudit', () => { { message: 'Type is module', severity: 'info', - source: { - file: 'package.json', - }, }, ], }, @@ -71,9 +68,6 @@ describe('typeAudit', () => { { message: 'Type is commonjs', severity: 'info', - source: { - file: 'package.json', - }, }, ], }, @@ -91,9 +85,6 @@ describe('typeAudit', () => { { message: 'Type is undefined. Defaults to commonjs.', severity: 'info', - source: { - file: 'package.json', - }, }, ], }, @@ -110,14 +101,8 @@ describe('typeAudit', () => { details: { issues: [ { - message: `type should be undefined or commonjs but is `, + message: 'type should be undefined or commonjs but is ', severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, @@ -134,11 +119,8 @@ describe('typeAudit', () => { details: { issues: [ { - message: `type should be module but is undefined`, + message: 'type should be module but is undefined', severity: 'error', - source: { - file: 'package.json', - }, }, ], }, @@ -155,14 +137,8 @@ describe('typeAudit', () => { details: { issues: [ { - message: `type empty`, + message: 'type empty', severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, @@ -183,12 +159,6 @@ describe('typeAudit', () => { { message: `type should be module but is ${targetPackageJson.type}`, severity: 'error', - source: { - file: 'package.json', - position: { - startLine: 1, - }, - }, }, ], }, diff --git a/examples/plugins/src/package-json/src/integration/utils.ts b/examples/plugins/src/package-json/src/integration/utils.ts index 6d8886c3e..a3d5d2695 100644 --- a/examples/plugins/src/package-json/src/integration/utils.ts +++ b/examples/plugins/src/package-json/src/integration/utils.ts @@ -1,9 +1,5 @@ import { AuditOutput, Issue } from '@code-pushup/models'; -import { - factorOf, - findLineNumberInText, - pluralizeToken, -} from '@code-pushup/utils'; +import { factorOf, pluralizeToken } from '@code-pushup/utils'; import { PackageJson, SourceResult } from './types'; export function baseAuditOutput(slug: string): AuditOutput { @@ -27,17 +23,13 @@ export function assertPropertyEmpty( result: SourceResult, property: keyof PackageJson, ): Issue { - const { file, content, json } = result; + const { json } = result; const value = json[property] as string | undefined; - const source: Issue['source'] = { - file, - }; if (value === undefined) { return { message: `${property} undefined`, severity: 'error', - source, }; } @@ -45,19 +37,12 @@ export function assertPropertyEmpty( return { message: `${property} empty`, severity: 'error', - source: { - ...source, - position: { - startLine: findLineNumberInText(content, `"${property}":`) as number, - }, - }, }; } return { message: `${property} OK`, severity: 'info', - source, }; } @@ -66,29 +51,18 @@ export function assertPropertyEqual( property: keyof PackageJson, value: unknown, ): Issue { - const { file, content, json } = result; + const { json } = result; if (json[property] !== value) { - const startLine: null | number = findLineNumberInText( - content, - `"${property}":`, - ); return { severity: 'error', message: `${property} should be ${value?.toString()} but is ${json[ property ]?.toString()}`, - source: { - file, - ...(startLine == null ? {} : { position: { startLine } }), - }, }; } return { message: `${property} value is given`, severity: 'info', - source: { - file, - }, }; } diff --git a/examples/plugins/src/package-json/src/package-json.plugin.integration.test.ts b/examples/plugins/src/package-json/src/package-json.plugin.integration.test.ts index 9a6a2a1af..7a262c0f6 100644 --- a/examples/plugins/src/package-json/src/package-json.plugin.integration.test.ts +++ b/examples/plugins/src/package-json/src/package-json.plugin.integration.test.ts @@ -76,9 +76,6 @@ describe('create-package-json', () => { { message: 'license should be MIT but is undefined', severity: 'error', - source: { - file: expect.stringContaining('package.json'), - }, }, ]); }); @@ -96,9 +93,6 @@ describe('create-package-json', () => { { message: 'type should be module but is undefined', severity: 'error', - source: { - file: expect.stringContaining('package.json'), - }, }, ]); }); @@ -120,9 +114,6 @@ describe('create-package-json', () => { message: 'Package test is not installed under dependencies. Run `npm install test@0` to install it.', severity: 'error', - source: { - file: expect.stringContaining('package.json'), - }, }, ]); }); @@ -144,9 +135,6 @@ describe('create-package-json', () => { message: 'Package test is not installed under optionalDependencies. Run `npm install test@0` to install it.', severity: 'error', - source: { - file: expect.stringContaining('package.json'), - }, }, ]); }); @@ -168,9 +156,6 @@ describe('create-package-json', () => { message: 'Package test is not installed under devDependencies. Run `npm install test@0` to install it.', severity: 'error', - source: { - file: expect.stringContaining('package.json'), - }, }, ]); });