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.

Sign up for GitHub

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

feat: add summary to action job #62

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add summary to action job #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9e0a927
feat: add job summary
michael-siek Feb 13, 2025
9db43a0
red text
michael-siek Feb 13, 2025
5d298cb
add red modifiers
michael-siek Feb 13, 2025
46956d6
merge conflict
michael-siek Feb 13, 2025
36c606f
fix test
michael-siek Feb 13, 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
61 changes: 49 additions & 12 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43021,6 +43021,7 @@ ${pendingInterceptorsFormatter.format(pending)}
function lintFiles(files, apiKey, axeLinterUrl, linterConfig) {
return __awaiter(this, void 0, void 0, function* () {
let totalErrors = 0
const fileErrors = {}
for (const file of files) {
const fileContents = (0, fs_1.readFileSync)(file, 'utf8')
// Skip empty files
Expand Down Expand Up @@ -43060,20 +43061,56 @@ ${pendingInterceptorsFormatter.format(pending)}
}
const errors = result.report.errors
totalErrors += errors.length
// Report errors using GitHub annotations
for (const error of errors) {
core.error(
`${file}:${error.lineNumber} - ${error.ruleId} - ${error.description}`,
{
file,
startLine: error.lineNumber,
startColumn: error.column,
endColumn: error.endColumn,
title: 'Axe Linter'
}
)
if (errors.length > 0) {
fileErrors[file] = errors.map((error) => ({
line: error.lineNumber,
column: error.column,
endColumn: error.endColumn,
message: `${error.ruleId} - ${error.description}`,
ruleId: error.ruleId,
description: error.description
}))
}
}
// Create summary of all errors
core.summary.addHeading('Accessibility Linting Results').addBreak()
for (const [file, errors] of Object.entries(fileErrors)) {
if (errors.length > 0) {
// Add file heading
core.summary
.addHeading(
`❌ Error${(0, utils_1.pluralize)(errors.length)} in ${file}:`,
2
)
.addList(
errors.map(
(error) =>
`🔴 Line ${error.line}: ${error.ruleId} - ${error.description}`
)
)
.addBreak()
// Create GitHub annotations
for (const error of errors) {
core.error(
`${file}:${error.line} - ${error.ruleId} - ${error.description}`,
{
file,
startLine: error.line,
startColumn: error.column,
endColumn: error.endColumn,
title: 'Axe Linter'
}
)
}
}
}
// Add summary footer
yield core.summary
.addBreak()
.addRaw(
`Found ${totalErrors} accessibility issue${(0, utils_1.pluralize)(totalErrors)}`
)
.write()
core.debug(
`Found ${totalErrors} error${(0, utils_1.pluralize)(totalErrors)}`
)
Expand Down
179 changes: 179 additions & 0 deletions src/linter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('linter', () => {
let sandbox: sinon.SinonSandbox
let errorStub: sinon.SinonStub
let debugStub: sinon.SinonStub
let summaryStub: any
let readFileStub: sinon.SinonStub
let fetchStub: sinon.SinonStub

Expand All @@ -22,6 +23,16 @@ describe('linter', () => {
errorStub = sandbox.stub(core, 'error')
debugStub = sandbox.stub(core, 'debug')

// Create summary stub with chainable methods
summaryStub = {
addHeading: sandbox.stub().returnsThis(),
addBreak: sandbox.stub().returnsThis(),
addList: sandbox.stub().returnsThis(),
addRaw: sandbox.stub().returnsThis(),
write: sandbox.stub().resolves()
}
sandbox.stub(core, 'summary').value(summaryStub)

// Stub file system
readFileStub = sandbox.stub()
sandbox.replace(require('fs'), 'readFileSync', readFileStub)
Expand Down Expand Up @@ -364,5 +375,173 @@ describe('linter', () => {
assert.isTrue(scope.isDone(), 'API request should be made')
}
})

it('should create summary with grouped errors', async () => {
const files = ['app.js', 'index.html']
const apiKey = 'test-key'
const axeLinterUrl = 'https://test-linter.com'
const linterConfig = {}

// Setup file reads
readFileStub.withArgs('app.js', 'utf8').returns('const x = 1;')
readFileStub.withArgs('index.html', 'utf8').returns('<div>test</div>')

// Mock API responses
nock(axeLinterUrl)
.post('/lint-source', {
source: 'const x = 1;',
filename: 'app.js',
config: linterConfig
})
.reply(200, {
report: {
errors: [
{
ruleId: 'click-handler',
lineNumber: 1,
column: 1,
endColumn: 10,
description: 'Click handler should have keyboard equivalent'
}
]
}
})

nock(axeLinterUrl)
.post('/lint-source', {
source: '<div>test</div>',
filename: 'index.html',
config: linterConfig
})
.reply(200, {
report: {
errors: [
{
ruleId: 'color-contrast',
lineNumber: 1,
column: 1,
endColumn: 15,
description: 'Element has insufficient color contrast'
},
{
ruleId: 'aria-label',
lineNumber: 1,
column: 1,
endColumn: 15,
description: 'Element should have aria-label'
}
]
}
})

const errorCount = await lintFiles(
files,
apiKey,
axeLinterUrl,
linterConfig
)

// Verify error count
assert.equal(errorCount, 3, 'Should return correct total error count')

// Verify summary creation
assert.isTrue(
summaryStub.addHeading.calledWith('Accessibility Linting Results'),
'Should add main heading'
)

// Verify file sections
assert.isTrue(
summaryStub.addHeading.calledWith('❌ Error in app.js:', 2),
'Should add app.js section'
)

assert.isTrue(
summaryStub.addHeading.calledWith('❌ Errors in index.html:', 2),
'Should add index.html section'
)

// Verify error lists
const appJsErrors = [
'🔴 Line 1: click-handler - Click handler should have keyboard equivalent'
]
const indexHtmlErrors = [
'🔴 Line 1: color-contrast - Element has insufficient color contrast',
'🔴 Line 1: aria-label - Element should have aria-label'
]

assert.isTrue(
summaryStub.addList.calledWith(appJsErrors),
'Should add app.js errors to list'
)
assert.isTrue(
summaryStub.addList.calledWith(indexHtmlErrors),
'Should add index.html errors to list'
)

// Verify footer
assert.isTrue(
summaryStub.addRaw.calledWith('Found 3 accessibility issues'),
'Should add correct error count to summary'
)

// Verify GitHub annotations
assert.equal(
errorStub.callCount,
3,
'Should create three error annotations'
)
assert.isTrue(
errorStub.calledWith(
'app.js:1 - click-handler - Click handler should have keyboard equivalent',
{
file: 'app.js',
startLine: 1,
startColumn: 1,
endColumn: 10,
title: 'Axe Linter'
}
),
'Should create correct annotation for app.js'
)
})

it('should handle no errors in summary', async () => {
const files = ['clean.js']
const apiKey = 'test-key'
const axeLinterUrl = 'https://test-linter.com'
const linterConfig = {}

readFileStub.withArgs('clean.js', 'utf8').returns('const x = 1;')

nock(axeLinterUrl)
.post('/lint-source')
.reply(200, {
report: {
errors: []
}
})

const errorCount = await lintFiles(
files,
apiKey,
axeLinterUrl,
linterConfig
)

assert.equal(errorCount, 0, 'Should return zero errors')
assert.isTrue(
summaryStub.addHeading.calledWith('Accessibility Linting Results'),
'Should still create summary heading'
)
assert.isTrue(
summaryStub.addRaw.calledWith('Found 0 accessibility issues'),
'Should show zero issues in summary'
)
assert.isFalse(
errorStub.called,
'Should not create any error annotations'
)
})
})
})
61 changes: 48 additions & 13 deletions src/linter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as core from '@actions/core'
import { readFileSync } from 'fs'
import type { LinterResponse } from './types'
import type { LinterResponse, ErrorDetail } from './types'
import fetch from 'node-fetch'
import { pluralize } from './utils'

Expand All @@ -11,6 +11,7 @@ export async function lintFiles(
linterConfig: Record<string, unknown>
): Promise<number> {
let totalErrors = 0
const fileErrors: Record<string, ErrorDetail[]> = {}

for (const file of files) {
const fileContents = readFileSync(file, 'utf8')
Expand Down Expand Up @@ -52,22 +53,56 @@ export async function lintFiles(

const errors = result.report.errors
totalErrors += errors.length
if (errors.length > 0) {
fileErrors[file] = errors.map((error) => ({
line: error.lineNumber,
column: error.column,
endColumn: error.endColumn,
message: `${error.ruleId} - ${error.description}`,
ruleId: error.ruleId,
description: error.description
}))
}
}

// Create summary of all errors
core.summary.addHeading('Accessibility Linting Results').addBreak()

// Report errors using GitHub annotations
for (const error of errors) {
core.error(
`${file}:${error.lineNumber} - ${error.ruleId} - ${error.description}`,
{
file,
startLine: error.lineNumber,
startColumn: error.column,
endColumn: error.endColumn,
title: 'Axe Linter'
}
)
for (const [file, errors] of Object.entries(fileErrors)) {
if (errors.length > 0) {
// Add file heading
core.summary
.addHeading(`❌ Error${pluralize(errors.length)} in ${file}:`, 2)
.addList(
errors.map(
(error) =>
`🔴 Line ${error.line}: ${error.ruleId} - ${error.description}`
)
)
Comment on lines +76 to +81
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you could consider eslint stylish formatter style here? I guess this format will be more familiar to the community.

If it's not very late, I'd also use the the same format in action logs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can ask the team and see what their thoughts were on this. I don't see why this would be an issue

.addBreak()

// Create GitHub annotations
for (const error of errors) {
core.error(
`${file}:${error.line} - ${error.ruleId} - ${error.description}`,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file:line looks redundant here, GH annotation will display this info anyway since you provide it in the options

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is defined in both GH annotations but also in the workflow itself. Without the file and line number the error is not too helpful
Ref: #60

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you output errors in eslint stylish format, you don't need to manually create annotations, you can rely on GH Problem Matchers:

=>

  1. readable errors in workflow logs
  2. annotations without redundancy

{
file,
startLine: error.line,
startColumn: error.column,
endColumn: error.endColumn,
title: 'Axe Linter'
}
)
}
}
}

// Add summary footer
await core.summary
.addBreak()
.addRaw(`Found ${totalErrors} accessibility issue${pluralize(totalErrors)}`)
.write()

core.debug(`Found ${totalErrors} error${pluralize(totalErrors)}`)
return totalErrors
}
9 changes: 9 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ export type Core = Pick<
'getInput' | 'setOutput' | 'info' | 'setFailed' | 'debug'
>

export interface ErrorDetail {
line: number
message: string
column: number
endColumn: number
ruleId: string
description: string
}

export interface LinterError {
ruleId: string
lineNumber: number
Expand Down