Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collapse output to 20 items #160

Merged
merged 23 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e55b9fa
Implement diagnostic quota logic
daniilsapa Jan 2, 2025
f3bdce4
Move diagnostic collapsing logic to pretty-printer
daniilsapa Jan 4, 2025
9d837bd
Add severity to consideration when collapsing the report, refactor th…
daniilsapa Jan 6, 2025
43e7f83
Add a changeset
daniilsapa Jan 6, 2025
f22685c
Merge branch 'next' into feature/collapse-output
daniilsapa Jan 6, 2025
75db8ef
Fix conflicts in pnpm-lock.yaml
daniilsapa Jan 6, 2025
66bbf39
Fix vitest version for pretty-reporter
daniilsapa Jan 6, 2025
8d3d231
Fix pnpm-lock issue
daniilsapa Jan 6, 2025
a58aa82
Count all autofixable diagnostics
daniilsapa Jan 11, 2025
b1df9d4
Update packages/pretty-reporter/src/index.ts
daniilsapa Jan 11, 2025
b9f7f58
Use s function for conditional pluralization
daniilsapa Jan 11, 2025
75d2b77
Fix the test case, remove unnecessary condition
daniilsapa Jan 11, 2025
42bfc21
Remove the unnecessary wrapper function, refactor test cases
daniilsapa Jan 11, 2025
d466eb9
Try to fix the size workflows
daniilsapa Jan 11, 2025
e98db05
Remove artifact duplicate
daniilsapa Jan 11, 2025
2e10bde
Use while loop instead of for
daniilsapa Jan 11, 2025
742a4f0
Fix sanitization command syntax
daniilsapa Jan 11, 2025
d43c8cc
Fix a typo in the workflow code
daniilsapa Jan 11, 2025
57ef178
Fix the issue with the loop
daniilsapa Jan 11, 2025
7566326
Make pretty-reporter test run along with the others
daniilsapa Jan 11, 2025
22f6650
Fix the size workout
daniilsapa Jan 11, 2025
84a40a9
Simplify the condition, remove the unnecessary comment
daniilsapa Jan 16, 2025
f3b4263
Add branch name sanitization to the trusted part of the size workflow
daniilsapa Jan 16, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/clever-moose-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@steiger/pretty-reporter': minor
---

Collapse the report to 20 diagnostics, add message about hidden diagnostics
6 changes: 5 additions & 1 deletion .github/workflows/bundle-size-trusted.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ jobs:
github-token: ${{ secrets.GITHUB_TOKEN }}
run-id: ${{ github.event.workflow_run.id }}

- name: Sanitize branch name
id: sanitize_branch
run: echo "sanitized=$(echo ${{ matrix.branch }} | sed 's/[^0-9a-zA-Z\._\-]/_/g')" >> $GITHUB_OUTPUT

- name: Create the report
id: create-report
uses: actions/github-script@v7
Expand All @@ -48,7 +52,7 @@ jobs:
}
const fs = require('fs');
const sizes = parseDuOutput(fs.readFileSync(`sizes-${${{ toJson(github.base_ref) }}}.txt`, 'utf8'));
const sizesPR = parseDuOutput(fs.readFileSync(`sizes-${${{ toJson(github.head_ref) }}}.txt`, 'utf8'));
const sizesPR = parseDuOutput(fs.readFileSync(`sizes-${${{ steps.sanitize_branch.outputs.sanitized }}}.txt`, 'utf8'));
illright marked this conversation as resolved.
Show resolved Hide resolved
core.summary.addHeading('📊 Package size report', '3');
core.summary.addTable([
['Package', 'Before', 'After'].map((data) => ({ data, header: true })),
Expand Down
10 changes: 7 additions & 3 deletions .github/workflows/bundle-size-untrusted.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ jobs:
with:
ref: ${{ matrix.branch }}

- name: Sanitize branch name
id: sanitize_branch
run: echo "sanitized=$(echo ${{ matrix.branch }} | sed 's/[^0-9a-zA-Z\._\-]/_/g')" >> $GITHUB_OUTPUT

- name: Setup
uses: ./.github/actions/setup

Expand All @@ -35,10 +39,10 @@ jobs:

- name: Collect sizes in bytes
id: sizes
run: du -sb packages/*/dist > sizes-${{ matrix.branch }}.txt
run: du -sb packages/*/dist > sizes-${{ steps.sanitize_branch.outputs.sanitized }}.txt

- name: Upload the sizes
uses: actions/upload-artifact@v4
with:
name: sizes-${{ matrix.branch }}
path: sizes-${{ matrix.branch }}.txt
name: sizes-${{ steps.sanitize_branch.outputs.sanitized }}
path: sizes-${{ steps.sanitize_branch.outputs.sanitized }}.txt
6 changes: 4 additions & 2 deletions packages/pretty-reporter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"lint": "eslint .",
"format": "prettier --write . --cache",
"check-formatting": "prettier --check . --cache",
"typecheck": "tsc --noEmit"
"typecheck": "tsc --noEmit",
"test": "vitest run"
},
"exports": {
".": "./src/index.ts"
Expand Down Expand Up @@ -42,6 +43,7 @@
"chalk": "^5.3.0",
"figures": "^6.1.0",
"picocolors": "^1.1.1",
"terminal-link": "^3.0.0"
"terminal-link": "^3.0.0",
"vitest": "^3.0.0-beta.2"
}
}
90 changes: 90 additions & 0 deletions packages/pretty-reporter/src/collapse-diagnostics.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { describe, expect, it } from 'vitest'
import { Diagnostic } from '@steiger/types'

import { collapseDiagnostics } from './collapse-diagnostics.js'

function makeDummyDiagnostic(rule: string, messageNum: number, severity?: 'warn' | 'error'): Diagnostic {
const defaultLocation = {
path: '/users/user/file',
}
const defaultSeverity = 'error' as 'warn' | 'error'

return {
message: `${rule}, message ${messageNum}`,
ruleName: rule,
location: defaultLocation,
severity: severity ?? defaultSeverity,
}
}

describe('trimDiagnosticsToMeetQuota', () => {
it('should return the same diagnostics if they are below the quota', () => {
const diagnosticPerRule = [
new Array(3).fill(0).map((_, i) => makeDummyDiagnostic('rule-1', i + 1)),
new Array(2).fill(0).map((_, i) => makeDummyDiagnostic('rule-2', i + 1)),
]

const result = collapseDiagnostics(diagnosticPerRule)

expect(result).toEqual(diagnosticPerRule)
})

it('should spread the quota evenly between rules', () => {
const diagnosticPerRule = [
new Array(20).fill(0).map((_, i) => makeDummyDiagnostic('rule-1', i + 1)),
new Array(20).fill(0).map((_, i) => makeDummyDiagnostic('rule-2', i + 1)),
]

const result = collapseDiagnostics(diagnosticPerRule)

expect(result).toEqual([diagnosticPerRule[0].slice(0, 10), diagnosticPerRule[1].slice(0, 10)])
})

it('should take first x rule diagnostics if the quota is less than the number of rules', () => {
const diagnosticPerRule = [
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-1', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-2', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-3', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-4', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-5', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-6', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-7', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-8', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-9', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-10', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-11', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-12', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-13', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-14', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-15', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-16', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-17', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-18', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-19', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-20', i + 1)),
new Array(1).fill(0).map((_, i) => makeDummyDiagnostic('rule-21', i + 1)),
]

const result = collapseDiagnostics(diagnosticPerRule)

expect(result).toEqual([...diagnosticPerRule.slice(0, 20), []])
})

it('should take errors first, then warnings', () => {
const diagnosticPerRule = [
new Array(10).fill(0).map((_, i) => makeDummyDiagnostic('rule-1', i + 1, i < 5 ? 'warn' : 'error')),
new Array(20).fill(0).map((_, i) => makeDummyDiagnostic('rule-2', i + 1, i >= 10 ? 'warn' : 'error')),
new Array(10).fill(0).map((_, i) => makeDummyDiagnostic('rule-3', i + 1, i < 5 ? 'warn' : 'error')),
]

const result = collapseDiagnostics(diagnosticPerRule)

expect(result).toEqual([
// errors items first, then warnings
[...diagnosticPerRule[0].slice(5), ...diagnosticPerRule[0].slice(0, 2)],
[...diagnosticPerRule[1].slice(0, 7)],
// error items first, then warnings
[...diagnosticPerRule[2].slice(5), ...diagnosticPerRule[2].slice(0, 1)],
])
})
})
44 changes: 44 additions & 0 deletions packages/pretty-reporter/src/collapse-diagnostics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { Diagnostic } from '@steiger/types'

const DIAGNOSTIC_QUOTA = 20

function distributeQuota(buckets: Array<number>, quota: number) {
const allItemsCount = buckets.reduce((acc, bucket) => acc + bucket, 0)
const quotaPerBucket = buckets.slice(0).fill(0)
let remainingQuota = Math.min(quota, allItemsCount)
let i = 0

while (remainingQuota > 0) {
const numOfItemsInBucket = buckets[i]
const assignedQuotaForBucket = quotaPerBucket[i]

// If it went beyond the last bucket, start from the first one
if (numOfItemsInBucket === undefined) {
i = 0
continue
}

if (assignedQuotaForBucket < numOfItemsInBucket) {
quotaPerBucket[i] += 1
remainingQuota -= 1
}

i += 1
}

return quotaPerBucket
}

function sortDiagnostics(diagnostics: Diagnostic[]): Diagnostic[] {
const severityOrder = { error: 1, warn: 2 }

return diagnostics.slice(0).sort((a, b) => severityOrder[a.severity] - severityOrder[b.severity])
}

export function collapseDiagnostics(diagnosticsPerRule: Array<Array<Diagnostic>>) {
const diagnosticCountPerRule = diagnosticsPerRule.map((diagnostics) => diagnostics.length)
const quotaPerRule = distributeQuota(diagnosticCountPerRule, DIAGNOSTIC_QUOTA)
const sortedDiagnosticsPerRule = diagnosticsPerRule.map((diagnostics) => sortDiagnostics(diagnostics))

return sortedDiagnosticsPerRule.map((diagnostics, i) => diagnostics.slice(0, quotaPerRule[i]))
}
74 changes: 74 additions & 0 deletions packages/pretty-reporter/src/group-diagnostics-by-rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { Diagnostic } from '@steiger/types'

export function groupDiagnosticsByRule(diagnostics: Diagnostic[]): Diagnostic[][] {
const grouped: Record<string, Diagnostic[]> = {}

diagnostics.forEach((diagnostic) => {
if (!grouped[diagnostic.ruleName]) {
grouped[diagnostic.ruleName] = []
}
grouped[diagnostic.ruleName].push(diagnostic)
})

return Object.values(grouped)
}

if (import.meta.vitest) {
const { describe, it, expect } = import.meta.vitest

describe('groupDiagnosticsByRule', () => {
it('should group diagnostics by ruleName', () => {
const diagnostics: Diagnostic[] = [
{
ruleName: 'rule-1',
severity: 'error',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'First rule, first message',
},
{
ruleName: 'rule-2',
severity: 'warn',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'Second rule, first message',
},
{
ruleName: 'rule-1',
severity: 'warn',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'First rule, second message',
},
]

const result = groupDiagnosticsByRule(diagnostics)

expect(result).toHaveLength(2)
expect(result).toContainEqual([
{
ruleName: 'rule-1',
severity: 'error',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'First rule, first message',
},
{
ruleName: 'rule-1',
severity: 'warn',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'First rule, second message',
},
])
expect(result).toContainEqual([
{
ruleName: 'rule-2',
severity: 'warn',
location: { path: '/users/user/project/src/app/index.ts' },
message: 'Second rule, first message',
},
])
})

it('should return an empty array when no diagnostics are provided', () => {
const result = groupDiagnosticsByRule([])
expect(result).toEqual([])
})
})
}
16 changes: 13 additions & 3 deletions packages/pretty-reporter/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@ import figures from 'figures'
import type { Diagnostic } from '@steiger/types'

import { formatSingleDiagnostic } from './format-single-diagnostic.js'
import { collapseDiagnostics } from './collapse-diagnostics.js'
import { groupDiagnosticsByRule } from './group-diagnostics-by-rule.js'
import { s } from './pluralization.js'

export function formatPretty(diagnostics: Array<Diagnostic>, cwd: string) {
if (diagnostics.length === 0) {
return pc.green(`${figures.tick} No problems found!`)
}

const collapsedDiagnostics = collapseDiagnostics(groupDiagnosticsByRule(diagnostics)).flat()
const collapsedDiagnosticsCount = collapsedDiagnostics.length
const initialDiagnosticsCount = diagnostics.length
const diffCount = initialDiagnosticsCount - collapsedDiagnosticsCount

const errors = diagnostics.filter((d) => d.severity === 'error')
const warnings = diagnostics.filter((d) => d.severity === 'warn')

Expand All @@ -23,7 +30,7 @@ export function formatPretty(diagnostics: Array<Diagnostic>, cwd: string) {
.join(' and ')

const autofixable = diagnostics.filter((d) => (d.fixes?.length ?? 0) > 0)
if (autofixable.length === diagnostics.length) {
if (autofixable.length === initialDiagnosticsCount) {
footer += ` (all can be fixed automatically with ${pc.bold(pc.green('--fix'))})`
} else if (autofixable.length > 0) {
footer += ` (${autofixable.length} can be fixed automatically with ${pc.bold(pc.green('--fix'))})`
Expand All @@ -33,13 +40,16 @@ export function formatPretty(diagnostics: Array<Diagnostic>, cwd: string) {

return (
'\n' +
diagnostics.map((d) => formatSingleDiagnostic(d, cwd)).join('\n\n') +
collapsedDiagnostics.map((d) => formatSingleDiagnostic(d, cwd)).join('\n\n') +
'\n\n' +
// Due to formatting characters, it won't be exactly the size of the footer, that is okay
pc.gray(figures.line.repeat(footer.length)) +
'\n ' +
footer +
'\n'
'\n ' +
(collapsedDiagnosticsCount < initialDiagnosticsCount
? `${pc.reset(diffCount)} diagnostic${s(diffCount)} are not shown, use ${pc.bold(pc.green('--reporter json'))} to see them.`
: '')
)
}

Expand Down
3 changes: 2 additions & 1 deletion packages/pretty-reporter/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"extends": "@steiger/tsconfig/base.json",
"include": ["./src", "./reset.d.ts", "example/index.ts"],
"compilerOptions": {
"tsBuildInfoFile": "node_modules/.cache/tsbuildinfo.json"
"tsBuildInfoFile": "node_modules/.cache/tsbuildinfo.json",
"types": ["vitest/importMeta"]
}
}
7 changes: 7 additions & 0 deletions packages/pretty-reporter/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'vitest/config'

export default defineConfig({
test: {
includeSource: ['src/**/*.{js,ts}'],
},
})
Loading
Loading