Skip to content

Commit 7e6c29d

Browse files
hanna-skrylhanna-skryl
authoredNov 8, 2024··
refactor: standardize undefined handling for categories
1 parent 01724d3 commit 7e6c29d

37 files changed

+211
-123
lines changed
 

‎packages/ci/src/lib/issues.ts

+22-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type {
22
Audit,
33
AuditReport,
4+
CategoryRef,
45
Issue,
56
PluginMeta,
67
Report,
@@ -151,6 +152,9 @@ export function getAuditImpactValue(
151152
{ audit, plugin }: IssueContext,
152153
report: Report,
153154
): number {
155+
if (!report.categories) {
156+
return 0;
157+
}
154158
return report.categories
155159
.map((category): number => {
156160
const weights = category.refs.map((ref): number => {
@@ -163,16 +167,7 @@ export function getAuditImpactValue(
163167
return ref.slug === audit.slug ? ref.weight : 0;
164168

165169
case 'group':
166-
const group = report.plugins
167-
.find(({ slug }) => slug === ref.plugin)
168-
?.groups?.find(({ slug }) => slug === ref.slug);
169-
if (!group?.refs.length) {
170-
return 0;
171-
}
172-
const groupRatio =
173-
(group.refs.find(({ slug }) => slug === audit.slug)?.weight ??
174-
0) / group.refs.reduce((acc, { weight }) => acc + weight, 0);
175-
return ref.weight * groupRatio;
170+
return calculateGroupImpact(ref, audit, report);
176171
}
177172
});
178173

@@ -183,3 +178,20 @@ export function getAuditImpactValue(
183178
})
184179
.reduce((acc, value) => acc + value, 0);
185180
}
181+
182+
export function calculateGroupImpact(
183+
ref: CategoryRef,
184+
audit: Audit,
185+
report: Report,
186+
): number {
187+
const group = report.plugins
188+
.find(({ slug }) => slug === ref.plugin)
189+
?.groups?.find(({ slug }) => slug === ref.slug);
190+
if (!group?.refs.length) {
191+
return 0;
192+
}
193+
const groupRatio =
194+
(group.refs.find(({ slug }) => slug === audit.slug)?.weight ?? 0) /
195+
group.refs.reduce((acc, { weight }) => acc + weight, 0);
196+
return ref.weight * groupRatio;
197+
}

‎packages/ci/src/lib/issues.unit.test.ts

+72-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
import type { Report } from '@code-pushup/models';
2-
import { getAuditImpactValue, issuesMatch } from './issues';
1+
import type { CategoryRef, Report } from '@code-pushup/models';
2+
import {
3+
calculateGroupImpact,
4+
getAuditImpactValue,
5+
issuesMatch,
6+
} from './issues';
37

48
describe('issues comparison', () => {
59
it('should match issues with exact same metadata', () => {
@@ -271,4 +275,70 @@ describe('issues sorting', () => {
271275
),
272276
).toBe(0.09); // 1% + 8% = 9%
273277
});
278+
279+
it('should return 0 when there are no categories', () => {
280+
expect(
281+
getAuditImpactValue(
282+
{
283+
audit: {
284+
slug: 'react-jsx-key',
285+
title: 'Disallow missing `key` props in iterators',
286+
},
287+
plugin: { slug: 'eslint', title: 'ESLint' },
288+
},
289+
{
290+
plugins: [
291+
{
292+
slug: 'eslint',
293+
groups: [
294+
{
295+
slug: 'suggestions',
296+
refs: [{ slug: 'mock-rule', weight: 1 }],
297+
},
298+
],
299+
},
300+
],
301+
} as Report,
302+
),
303+
).toBe(0);
304+
});
305+
});
306+
307+
describe('calculateGroupImpact', () => {
308+
const mockAudit = {
309+
slug: 'react-jsx-key',
310+
title: 'Disallow missing `key` props in iterators',
311+
};
312+
const mockCategoryRef = {
313+
type: 'group',
314+
plugin: 'eslint',
315+
slug: 'suggestions',
316+
weight: 1,
317+
} as CategoryRef;
318+
319+
const mockReport = {
320+
plugins: [
321+
{
322+
slug: 'eslint',
323+
groups: [
324+
{
325+
slug: 'suggestions',
326+
refs: [
327+
...Array.from({ length: 9 }).map((_, i) => ({
328+
slug: `mock-rule-${i}`,
329+
weight: 1,
330+
})),
331+
{ slug: 'react-jsx-key', weight: 1 },
332+
],
333+
},
334+
],
335+
},
336+
],
337+
} as Report;
338+
339+
it('should calculate correct impact for audit in group', () => {
340+
expect(calculateGroupImpact(mockCategoryRef, mockAudit, mockReport)).toBe(
341+
0.1,
342+
);
343+
});
274344
});

‎packages/cli/src/lib/autorun/autorun-command.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export function yargsAutorunCommandObject() {
4141
await collectAndPersistReports(optionsWithFormat);
4242
collectSuccessfulLog();
4343

44-
if (options.categories.length === 0) {
44+
if (!options.categories || options.categories.length === 0) {
4545
renderConfigureCategoriesHint();
4646
}
4747

‎packages/cli/src/lib/collect/collect-command.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function yargsCollectCommandObject(): CommandModule {
2424
await collectAndPersistReports(options);
2525
collectSuccessfulLog();
2626

27-
if (options.categories.length === 0) {
27+
if (!options.categories || options.categories.length === 0) {
2828
renderConfigureCategoriesHint();
2929
}
3030

‎packages/cli/src/lib/implementation/core-config.integration.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ describe('parsing values from CLI and middleware', () => {
144144
});
145145
});
146146

147-
it('should set empty array for categories if not given in config file', async () => {
147+
it('should keep categories undefined if not given in config file', async () => {
148148
const { categories } = await yargsCli<CoreConfig>(
149149
['--config=./no-category.config.ts'],
150150
{
@@ -153,7 +153,7 @@ describe('parsing values from CLI and middleware', () => {
153153
},
154154
).parseAsync();
155155

156-
expect(categories).toEqual([]);
156+
expect(categories).toBeUndefined();
157157
});
158158

159159
it('should accept an upload configuration with CLI overrides', async () => {

‎packages/cli/src/lib/implementation/core-config.middleware.ts

-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ export async function coreConfigMiddleware<
3232
const {
3333
persist: rcPersist,
3434
upload: rcUpload,
35-
categories: rcCategories,
3635
...remainingRcConfig
3736
} = importedRc;
3837
const upload =
@@ -56,7 +55,6 @@ export async function coreConfigMiddleware<
5655
),
5756
},
5857
...(upload != null && { upload }),
59-
categories: rcCategories ?? [],
6058
...remainingRcConfig,
6159
...remainingCliOptions,
6260
};

‎packages/cli/src/lib/implementation/filter.middleware.ts

+20-13
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { CategoryConfig, PluginConfig } from '@code-pushup/models';
1+
import type { CoreConfig } from '@code-pushup/models';
22
import { filterItemRefsBy } from '@code-pushup/utils';
33
import type { FilterOptions, Filterables } from './filter.model';
44
import {
@@ -12,7 +12,7 @@ export function filterMiddleware<T extends FilterOptions>(
1212
): T {
1313
const {
1414
plugins,
15-
categories = [],
15+
categories,
1616
skipCategories = [],
1717
onlyCategories = [],
1818
skipPlugins = [],
@@ -26,7 +26,7 @@ export function filterMiddleware<T extends FilterOptions>(
2626
skipPlugins.length === 0 &&
2727
onlyPlugins.length === 0
2828
) {
29-
return { ...originalProcessArgs, categories };
29+
return originalProcessArgs;
3030
}
3131

3232
handleConflictingOptions('categories', onlyCategories, skipCategories);
@@ -44,9 +44,11 @@ export function filterMiddleware<T extends FilterOptions>(
4444
onlyPlugins,
4545
verbose,
4646
);
47-
const finalCategories = filterItemRefsBy(filteredCategories, ref =>
48-
filteredPlugins.some(plugin => plugin.slug === ref.plugin),
49-
);
47+
const finalCategories = filteredCategories
48+
? filterItemRefsBy(filteredCategories, ref =>
49+
filteredPlugins.some(plugin => plugin.slug === ref.plugin),
50+
)
51+
: filteredCategories;
5052

5153
validateFinalState(
5254
{ categories: finalCategories, plugins: filteredPlugins },
@@ -80,8 +82,12 @@ function applyCategoryFilters(
8082
skipCategories: string[],
8183
onlyCategories: string[],
8284
verbose: boolean,
83-
): CategoryConfig[] {
84-
if (skipCategories.length === 0 && onlyCategories.length === 0) {
85+
): CoreConfig['categories'] {
86+
if (
87+
(skipCategories.length === 0 && onlyCategories.length === 0) ||
88+
!categories ||
89+
categories.length === 0
90+
) {
8591
return categories;
8692
}
8793
validateFilterOption(
@@ -102,7 +108,7 @@ function applyPluginFilters(
102108
skipPlugins: string[],
103109
onlyPlugins: string[],
104110
verbose: boolean,
105-
): PluginConfig[] {
111+
): CoreConfig['plugins'] {
106112
const filteredPlugins = filterPluginsFromCategories({
107113
categories,
108114
plugins,
@@ -126,11 +132,12 @@ function applyPluginFilters(
126132
function filterPluginsFromCategories({
127133
categories,
128134
plugins,
129-
}: Filterables): PluginConfig[] {
135+
}: Filterables): CoreConfig['plugins'] {
136+
if (!categories || categories.length === 0) {
137+
return plugins;
138+
}
130139
const validPluginSlugs = new Set(
131140
categories.flatMap(category => category.refs.map(ref => ref.plugin)),
132141
);
133-
return categories.length > 0
134-
? plugins.filter(plugin => validPluginSlugs.has(plugin.slug))
135-
: plugins;
142+
return plugins.filter(plugin => validPluginSlugs.has(plugin.slug));
136143
}

‎packages/cli/src/lib/implementation/filter.middleware.unit.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ describe('filterMiddleware', () => {
2222
}),
2323
).toStrictEqual({
2424
plugins: [{ slug: 'p1', audits: [{ slug: 'a1-p1' }] }],
25-
categories: [],
2625
});
2726
});
2827

@@ -89,7 +88,6 @@ describe('filterMiddleware', () => {
8988
const { plugins } = filterMiddleware({
9089
...option,
9190
plugins: [{ slug: 'p1' }, { slug: 'p2' }] as PluginConfig[],
92-
categories: [],
9391
});
9492
expect(plugins).toStrictEqual([expect.objectContaining(expected)]);
9593
},

‎packages/cli/src/lib/implementation/filter.model.ts

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import type { GlobalOptions } from '@code-pushup/core';
2-
import type {
3-
CategoryConfig,
4-
CoreConfig,
5-
PluginConfig,
6-
} from '@code-pushup/models';
2+
import type { CoreConfig } from '@code-pushup/models';
73

84
export type FilterOptions = Partial<GlobalOptions> &
95
Pick<CoreConfig, 'categories' | 'plugins'> &
@@ -17,7 +13,4 @@ export type FilterOptionType =
1713
| 'skipPlugins'
1814
| 'onlyPlugins';
1915

20-
export type Filterables = {
21-
categories: CategoryConfig[];
22-
plugins: PluginConfig[];
23-
};
16+
export type Filterables = Pick<CoreConfig, 'plugins' | 'categories'>;

‎packages/cli/src/lib/implementation/validate-filter-options.utils.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export class OptionValidationError extends Error {}
1111

1212
export function validateFilterOption(
1313
option: FilterOptionType,
14-
{ plugins, categories }: Filterables,
14+
{ plugins, categories = [] }: Filterables,
1515
{ itemsToFilter, verbose }: { itemsToFilter: string[]; verbose: boolean },
1616
): void {
1717
const itemsToFilterSet = new Set(itemsToFilter);
@@ -59,9 +59,9 @@ export function validateFinalState(
5959
filteredItems: Filterables,
6060
originalItems: Filterables,
6161
): void {
62-
const { categories: filteredCategories, plugins: filteredPlugins } =
62+
const { categories: filteredCategories = [], plugins: filteredPlugins } =
6363
filteredItems;
64-
const { categories: originalCategories, plugins: originalPlugins } =
64+
const { categories: originalCategories = [], plugins: originalPlugins } =
6565
originalItems;
6666
if (
6767
filteredCategories.length === 0 &&

‎packages/cli/src/lib/implementation/validate-filter-options.utils.unit.test.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ describe('validateFilterOption', () => {
106106
{ slug: 'p1', audits: [{ slug: 'a1-p1' }] },
107107
{ slug: 'p2', audits: [{ slug: 'a1-p2' }] },
108108
] as PluginConfig[],
109-
categories: [],
110109
},
111110
{ itemsToFilter: ['p1'], verbose: false },
112111
);
@@ -145,7 +144,6 @@ describe('validateFilterOption', () => {
145144
{ slug: 'p2', audits: [{ slug: 'a1-p2' }] },
146145
{ slug: 'p3', audits: [{ slug: 'a1-p3' }] },
147146
] as PluginConfig[],
148-
categories: [],
149147
},
150148
{ itemsToFilter: ['p4', 'p5'], verbose: false },
151149
);
@@ -165,12 +163,12 @@ describe('validateFilterOption', () => {
165163
expect(() => {
166164
validateFilterOption(
167165
'skipPlugins',
168-
{ plugins: allPlugins, categories: [] },
166+
{ plugins: allPlugins },
169167
{ itemsToFilter: ['plugin1'], verbose: false },
170168
);
171169
validateFilterOption(
172170
'onlyPlugins',
173-
{ plugins: allPlugins, categories: [] },
171+
{ plugins: allPlugins },
174172
{ itemsToFilter: ['plugin3'], verbose: false },
175173
);
176174
}).toThrow(
@@ -320,6 +318,21 @@ describe('validateFinalState', () => {
320318
validateFinalState(filteredItems, originalItems);
321319
}).toThrow(expect.any(OptionValidationError));
322320
});
321+
322+
it('should perform validation without throwing an error when categories are missing', () => {
323+
const filteredItems = {
324+
plugins: [{ slug: 'p1', audits: [{ slug: 'a1-p1' }] }] as PluginConfig[],
325+
};
326+
const originalItems = {
327+
plugins: [
328+
{ slug: 'p1', audits: [{ slug: 'a1-p1' }] },
329+
{ slug: 'p2', audits: [{ slug: 'a1-p2' }] },
330+
] as PluginConfig[],
331+
};
332+
expect(() => {
333+
validateFinalState(filteredItems, originalItems);
334+
}).not.toThrow();
335+
});
323336
});
324337

325338
describe('getItemType', () => {

‎packages/core/src/lib/collect-and-persist.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ import { collect } from './implementation/collect';
1313
import { logPersistedResults, persistReport } from './implementation/persist';
1414
import type { GlobalOptions } from './types';
1515

16-
export type CollectAndPersistReportsOptions = Required<
17-
Pick<CoreConfig, 'plugins' | 'categories'>
16+
export type CollectAndPersistReportsOptions = Pick<
17+
CoreConfig,
18+
'plugins' | 'categories'
1819
> & { persist: Required<PersistConfig> } & Partial<GlobalOptions>;
1920

2021
export async function collectAndPersistReports(

0 commit comments

Comments
 (0)