Skip to content

Commit

Permalink
refactor(examples-plugins): remove source links in issues
Browse files Browse the repository at this point in the history
  • Loading branch information
matejchalk committed Feb 9, 2024
1 parent db12855 commit 89f0a76
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 248 deletions.
13 changes: 2 additions & 11 deletions examples/plugins/src/file-size/src/file-size.plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
factorOf,
formatBytes,
pluralizeToken,
toUnixPath,
} from '@code-pushup/utils';

export type PluginOptions = {
Expand Down Expand Up @@ -165,32 +164,24 @@ 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<Issue, 'source'>;

if (budget !== undefined) {
// ensure budget is positive numbers
const formattedBudget = Math.max(budget, 0);
// 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;
55 changes: 4 additions & 51 deletions examples/plugins/src/file-size/src/file-size.plugin.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ describe('assertFileSize', () => {
expect(assertFileSize('test.js', size)).toEqual({
message: infoMessage('test.js', size),
severity: 'info',
source: { file: 'test.js' },
});
},
);
Expand All @@ -75,7 +74,6 @@ describe('assertFileSize', () => {
expect(assertFileSize('test.js', size, budget)).toEqual({
message: infoMessage('test.js', size),
severity: 'info',
source: { file: 'test.js' },
});
},
);
Expand All @@ -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' },
});
},
);
Expand All @@ -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),
})),
);
});

Expand All @@ -132,9 +124,6 @@ describe('fileSizeIssues', () => {
{
message: expect.any(String),
severity: expect.any(String),
source: {
file: expect.stringContaining('test.js'),
},
},
]);
});
Expand All @@ -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'),
},
},
]),
);
Expand All @@ -183,9 +163,6 @@ describe('fileSizeIssues', () => {
{
message: expect.any(String),
severity: 'error',
source: {
file: expect.stringContaining('test.js'),
},
},
]);
});
Expand Down Expand Up @@ -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({
Expand All @@ -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'),
},
},
]),
},
Expand Down
3 changes: 0 additions & 3 deletions examples/plugins/src/lighthouse/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ export function lhrDetailsToIssueDetails(
.join(',')
.slice(0, MAX_ISSUE_MESSAGE_LENGTH),
severity: 'info',
source: {
file: 'required-in-portal-api',
},
},
];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
)
Expand All @@ -91,50 +90,34 @@ export function dependenciesIssues(
}

export function packageNotInstalledIssue(
packageResult: Pick<SourceResult, 'file'>,
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(
packageResult: SourceResult,
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,
};
}
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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', () => {
Expand All @@ -45,9 +37,6 @@ describe('assertPackageVersion', () => {
).toEqual({
message: `Package ${packageName}@0.0.0 is installed as dependencies.`,
severity: 'info',
source: {
file: packageJsonName,
},
});
});

Expand All @@ -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,
},
},
});
});
});
Expand Down Expand Up @@ -106,9 +89,6 @@ describe('dependenciesAudit', () => {
{
message: `Package ${packageName}@${targetVersion} is installed as dependencies.`,
severity: 'info',
source: {
file: 'package.json',
},
},
],
},
Expand Down Expand Up @@ -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),
},
],
},
Expand Down Expand Up @@ -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,
},
},
},
],
},
Expand Down Expand Up @@ -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),
},
],
},
Expand Down
Loading

0 comments on commit 89f0a76

Please sign in to comment.