Skip to content

Commit 4769394

Browse files
feat: enhance report annotation action pr commenting and per-type limits
- Introduced per-type limits for annotations (error/warning/notice) to better manage output. - Updated action inputs and outputs in action.yml to reflect new functionality. - Enhanced error handling for PR comment creation and added comments for skipped annotations. - Updated dependencies to include @actions/github for improved GitHub API interactions.
1 parent 39f0605 commit 4769394

File tree

11 files changed

+4667
-271
lines changed

11 files changed

+4667
-271
lines changed

README.md

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ tests, linters, etc.
1212
## Usage Example
1313

1414
```yml
15+
permissions:
16+
pull-requests: write # Required for creating comments on skipped annotations
17+
1518
steps:
1619
- name: Checkout
1720
id: checkout
@@ -29,7 +32,6 @@ steps:
2932
junit|reports/junit-generic.xml
3033
junit-eslint|reports/*-eslint.xml
3134
junit-jest|reports/junit-jest.xml
32-
max-annotations: 20 # Keep the clutter down (50 is max by GitHub)
3335
ignore: node_modules/**,dist/** # Ignore patterns for the report search (default).
3436

3537
- name: Annotations created
@@ -41,6 +43,34 @@ steps:
4143
echo "Notices: ${{ steps.annotate.outputs.notices }}"
4244
```
4345
46+
## Inputs
47+
48+
| Name | Description | Default |
49+
| ----------------- | --------------------------------------------------------------------------------------------------------------------------------- | -------------------------------- |
50+
| `reports` | Reports to annotate: `"format\|glob1, glob2, ..."`<br>For example: `"junit-eslint\|junit/lint.xml"` | `["junit\|junit/*.xml"]` |
51+
| `ignore` | Ignore files from report search: `"[glob1, glob2...]"` | `['node_modules/**', 'dist/**']` |
52+
| `max-annotations` | Maximum number of annotations per type (error/warning/notice) | `10` |
53+
| `custom-matchers` | Custom matchers to use for parsing reports in JSON format: `{ "matcher-name": ReportMatcher }`<br>See ./src/matchers for examples | |
54+
| `token` | GitHub token for creating PR comments when annotations are skipped | `${{ github.token }}` |
55+
56+
## Skipped Annotations
57+
58+
When the maximum number of annotations per type is reached, additional
59+
annotations are not displayed as GitHub annotations to avoid clutter. Instead,
60+
they are added as a comment on the pull request using GitHub's markdown box
61+
syntax:
62+
63+
- **Error** boxes for skipped error annotations
64+
- **Warning** boxes for skipped warning annotations
65+
- **Note** boxes for skipped notice annotations
66+
67+
This ensures that all issues are still visible to developers without
68+
overwhelming the GitHub UI.
69+
70+
> [!NOTE] For more information about GitHub Actions annotation limitations, see
71+
> the
72+
> [official documentation](https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md#limitations).
73+
4474
> [!NOTE]
4575
>
4676
> You'll need to have a reasonably modern version of

__tests__/main.test.ts

Lines changed: 160 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { jest } from '@jest/globals';
22

3+
// Type for mutable context in tests
4+
type MutableContext = Omit<Context, 'payload' | 'repo'> & {
5+
payload: Context['payload'] & Record<string, unknown>;
6+
repo: Context['repo'];
7+
};
8+
39
// In Jest 30 with ESM, directly spying on re-exported ESM bindings of '@actions/core'
410
// and assigning implementations (jest.spyOn(core, 'debug').mockImplementation(...))
511
// can throw "Cannot assign to read only property" because the ESM named exports are
@@ -9,6 +15,7 @@ import { jest } from '@jest/globals';
915
// read-only export objects.
1016

1117
const coreMocks: Record<string, jest.Mock> = {};
18+
const githubMocks: Record<string, jest.Mock> = {};
1219

1320
await jest.unstable_mockModule('@actions/core', async () => {
1421
const original =
@@ -34,12 +41,35 @@ await jest.unstable_mockModule('@actions/core', async () => {
3441
} as typeof import('@actions/core');
3542
});
3643

44+
await jest.unstable_mockModule('@actions/github', async () => {
45+
const original =
46+
await jest.requireActual<typeof import('@actions/github')>(
47+
'@actions/github',
48+
);
49+
const make = <T extends keyof typeof original>(key: T) => {
50+
const fn = jest.fn();
51+
githubMocks[key as string] = fn;
52+
return fn;
53+
};
54+
return {
55+
...original,
56+
getOctokit: make('getOctokit'),
57+
context: {
58+
payload: {},
59+
repo: { owner: 'test-owner', repo: 'test-repo' },
60+
},
61+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
62+
} as any;
63+
});
64+
3765
// Dynamically import after mocking so the SUT picks up mocked module.
3866
const main = await import('../src/main');
67+
const github = await import('@actions/github');
68+
69+
type Context = typeof github.context;
3970

4071
// Mock the GitHub Actions core library
4172
// Logs & Annotations.
42-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
4373
let infoMock: jest.Mock;
4474
let errorMock: jest.Mock;
4575
let warningMock: jest.Mock;
@@ -49,10 +79,24 @@ let testInputs: Record<string, string | string[]>;
4979
// Outputs
5080
let setFailedMock: jest.Mock;
5181
let setOutputMock: jest.Mock;
82+
// GitHub API
83+
let mockOctokit: {
84+
rest: {
85+
issues: {
86+
createComment: jest.Mock;
87+
};
88+
};
89+
};
5290

5391
describe('action', () => {
5492
beforeEach(() => {
5593
jest.clearAllMocks();
94+
// Reset GitHub context
95+
(github.context as MutableContext).payload = {};
96+
(github.context as MutableContext).repo = {
97+
owner: 'test-owner',
98+
repo: 'test-repo',
99+
};
56100
// Simple logging mock (can redirect to stdout for debugging if desired)
57101
const logMock = jest.fn();
58102
coreMocks.debug.mockImplementation(logMock);
@@ -91,6 +135,15 @@ describe('action', () => {
91135
// Outputs
92136
setFailedMock = coreMocks.setFailed;
93137
setOutputMock = coreMocks.setOutput;
138+
// GitHub API
139+
mockOctokit = {
140+
rest: {
141+
issues: {
142+
createComment: jest.fn(),
143+
},
144+
},
145+
};
146+
githubMocks.getOctokit.mockReturnValue(mockOctokit);
94147
});
95148

96149
it('should parse report correctly', async () => {
@@ -220,26 +273,21 @@ at Tests.Registration.main(Registration.java:202)`,
220273
expect(setOutputMock).toHaveBeenCalledWith('total', 3);
221274
});
222275

223-
it('should respect maxAnnotations', async () => {
276+
it('should respect per-type annotation limits', async () => {
224277
testInputs.reports = ['junit|fixtures/junit-generic.xml'];
225-
testInputs['max-annotations'] = '1';
226278
await main.run();
227-
expect(warningMock).toHaveBeenCalledWith(
228-
'Maximum number of annotations reached (1). 2 annotations were not shown.',
229-
);
230-
expect(setOutputMock).toHaveBeenCalledWith('errors', 1);
279+
expect(setOutputMock).toHaveBeenCalledWith('errors', 2);
231280
expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
232-
expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
233-
expect(setOutputMock).toHaveBeenCalledWith('total', 1);
281+
expect(setOutputMock).toHaveBeenCalledWith('notices', 1);
282+
expect(setOutputMock).toHaveBeenCalledWith('total', 3);
234283
});
235284

236-
it('should prioritize errors over warnings and notices when maxAnnotations is reached', async () => {
285+
it('should show annotations up to per-type limits', async () => {
237286
// This test uses junit-eslint fixture which has both errors and warnings
238287
testInputs.reports = ['junit-eslint|fixtures/junit-eslint.xml'];
239-
testInputs['max-annotations'] = '1';
240288
await main.run();
241289

242-
// Should show the error first, not the warning
290+
// Should show the error
243291
expect(errorMock).toHaveBeenCalledWith(
244292
'["Bucket"] is better written in dot notation.',
245293
{
@@ -252,16 +300,16 @@ at Tests.Registration.main(Registration.java:202)`,
252300
},
253301
);
254302

255-
// Warning should not be called since we only allow 1 annotation and error has priority
256-
expect(warningMock).not.toHaveBeenCalledWith(
303+
// Warning should also be called since we allow 10 of each type
304+
expect(warningMock).toHaveBeenCalledWith(
257305
'Missing JSDoc comment.',
258306
expect.any(Object),
259307
);
260308

261309
expect(setOutputMock).toHaveBeenCalledWith('errors', 1);
262-
expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
310+
expect(setOutputMock).toHaveBeenCalledWith('warnings', 1);
263311
expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
264-
expect(setOutputMock).toHaveBeenCalledWith('total', 1);
312+
expect(setOutputMock).toHaveBeenCalledWith('total', 2);
265313
});
266314

267315
it('should support jest junit files', async () => {
@@ -288,4 +336,100 @@ at Tests.Registration.main(Registration.java:202)`,
288336
expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
289337
expect(setOutputMock).toHaveBeenCalledWith('total', 2);
290338
});
339+
340+
it('should handle unsupported matcher format', async () => {
341+
testInputs.reports = ['unsupported|fixtures/junit-generic.xml'];
342+
testInputs['custom-matchers'] = `{
343+
"unsupported": {
344+
"format": "json",
345+
"item": "//testcase",
346+
"message": "text()",
347+
"file": "@file"
348+
}
349+
}`;
350+
await expect(main.run()).rejects.toThrow(
351+
'Unsupported matcher format in unsupported: json',
352+
);
353+
});
354+
355+
it('should handle reports with no items', async () => {
356+
testInputs.reports = ['junit|fixtures/empty-report.xml'];
357+
await main.run();
358+
expect(errorMock).not.toHaveBeenCalled();
359+
expect(setOutputMock).toHaveBeenCalledWith('errors', 0);
360+
expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
361+
expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
362+
expect(setOutputMock).toHaveBeenCalledWith('total', 0);
363+
});
364+
365+
it('should handle yaml config file', async () => {
366+
testInputs.configPath = 'fixtures/test-config.yml';
367+
testInputs.reports = []; // Will use yaml config
368+
await main.run();
369+
expect(infoMock).toHaveBeenCalledWith(
370+
'Using config file at fixtures/test-config.yml',
371+
);
372+
});
373+
374+
it('should handle skipped annotations', async () => {
375+
testInputs.reports = ['junit|fixtures/junit-many-errors.xml'];
376+
testInputs['max-annotations'] = '2'; // Force skipping
377+
await main.run();
378+
expect(warningMock).toHaveBeenCalledWith(
379+
'Maximum number of annotations per type reached (2). 1 annotations were not shown.',
380+
);
381+
expect(setOutputMock).toHaveBeenCalledWith('errors', 2);
382+
expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
383+
expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
384+
expect(setOutputMock).toHaveBeenCalledWith('total', 2);
385+
});
386+
387+
it('should skip PR comment when not on a pull request', async () => {
388+
// Mock GitHub context to not be on a PR
389+
(github.context as MutableContext).payload = {};
390+
testInputs.reports = ['junit|fixtures/junit-many-errors.xml'];
391+
testInputs['max-annotations'] = '2';
392+
await main.run();
393+
expect(infoMock).toHaveBeenCalledWith(
394+
'Not running on a pull request, skipping comment creation.',
395+
);
396+
expect(mockOctokit.rest.issues.createComment).not.toHaveBeenCalled();
397+
});
398+
399+
it('should create PR comment when annotations are skipped', async () => {
400+
// Mock GitHub context to be on a PR
401+
(github.context as MutableContext).payload = {
402+
pull_request: { number: 123 },
403+
};
404+
testInputs.reports = ['junit|fixtures/junit-many-errors.xml'];
405+
testInputs['max-annotations'] = '2';
406+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
407+
(mockOctokit.rest.issues.createComment as any).mockResolvedValue({});
408+
await main.run();
409+
expect(mockOctokit.rest.issues.createComment).toHaveBeenCalledWith({
410+
owner: 'test-owner',
411+
repo: 'test-repo',
412+
issue_number: 123,
413+
body: expect.stringContaining('## Skipped Annotations'),
414+
});
415+
expect(infoMock).toHaveBeenCalledWith(
416+
'Created PR comment with skipped annotations.',
417+
);
418+
});
419+
420+
it('should handle PR comment API failure', async () => {
421+
// Mock GitHub context to be on a PR
422+
(github.context as MutableContext).payload = {
423+
pull_request: { number: 123 },
424+
};
425+
testInputs.reports = ['junit|fixtures/junit-many-errors.xml'];
426+
testInputs['max-annotations'] = '2';
427+
const apiError = new Error('API Error');
428+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
429+
(mockOctokit.rest.issues.createComment as any).mockRejectedValue(apiError);
430+
await main.run();
431+
expect(errorMock).toHaveBeenCalledWith(
432+
`Failed to create PR comment: ${apiError}`,
433+
);
434+
});
291435
});

action.yml

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,25 @@ inputs:
1111
description: |-
1212
Reports to annotate: "format|glob1, glob2, ..."
1313
For example: "junit-eslint|junit/lint.xml"
14-
Default: ["junit|junit/*.xml"]
14+
default: |
15+
junit|junit/*.xml
1516
ignore:
1617
description: |-
1718
Ignore files from report search: "[glob1, glob2...]"
18-
Default: ['node_modules', 'dist']
19+
default: |
20+
node_modules/**
21+
dist/**
1922
max-annotations:
2023
description: |-
21-
Maximum number of annotations to create
22-
Default: 50
24+
Maximum number of annotations per type (error/warning/notice)
25+
default: '10'
2326
custom-matchers:
2427
description: |-
2528
Custom matchers to use for parsing reports in JSON format: { "matcher-name": ReportMatcher }
2629
See ./src/matchers for examples
30+
token:
31+
description: GitHub token for creating PR comments
32+
default: ${{ github.token }}
2733

2834
outputs:
2935
errors:
@@ -33,7 +39,7 @@ outputs:
3339
notices:
3440
description: Notices found in reports
3541
total:
36-
description: Total annotations created (limited by max-annotations)
42+
description: Total annotations created
3743

3844
runs:
3945
using: node24

0 commit comments

Comments
 (0)