Skip to content

Commit 022dc9c

Browse files
authored
fix: avoid duplicate package suffix in Go benchmarks [#336] (#337)
- Fixes duplicate suffixes for users who worked around #264 by including package names in benchmark names - Detects when benchmark name already contains package reference (full path, underscored, or ≥2 trailing segments) and skips suffix - Adds go-force-package-suffix option to always append suffix regardless of suffix detection
1 parent 7e8372b commit 022dc9c

9 files changed

Lines changed: 253 additions & 3 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
name: github-issues
3+
description: Manage GitHub issues using the `gh` CLI. Use when the user asks to list, view, create, edit, close, reopen, comment on, or search GitHub issues.
4+
tools: Bash
5+
---
6+
7+
# GitHub Issues Skill
8+
9+
This project's repository is `benchmark-action/github-action-benchmark`. Always use `-R benchmark-action/github-action-benchmark` unless the user explicitly specifies a different repo.
10+
11+
## Common Operations
12+
13+
### List issues
14+
```bash
15+
gh issue list -R benchmark-action/github-action-benchmark
16+
gh issue list -R benchmark-action/github-action-benchmark --state all
17+
gh issue list -R benchmark-action/github-action-benchmark --state closed
18+
gh issue list -R benchmark-action/github-action-benchmark --label "bug"
19+
gh issue list -R benchmark-action/github-action-benchmark --assignee "@me"
20+
gh issue list -R benchmark-action/github-action-benchmark --author monalisa
21+
gh issue list -R benchmark-action/github-action-benchmark --search "error no:assignee"
22+
gh issue list -R benchmark-action/github-action-benchmark --limit 100
23+
gh issue list -R benchmark-action/github-action-benchmark --json number,title,state,labels,url
24+
```
25+
26+
### View an issue
27+
```bash
28+
gh issue view 123 -R benchmark-action/github-action-benchmark
29+
gh issue view 123 -R benchmark-action/github-action-benchmark --comments
30+
gh issue view 123 -R benchmark-action/github-action-benchmark --json title,body,comments,labels,assignees,state,url
31+
```
32+
33+
### Create an issue
34+
```bash
35+
gh issue create -R benchmark-action/github-action-benchmark --title "Title" --body "Body"
36+
gh issue create -R benchmark-action/github-action-benchmark --title "Bug" --label "bug" --assignee "@me"
37+
gh issue create -R benchmark-action/github-action-benchmark --web
38+
```
39+
40+
### Edit an issue
41+
```bash
42+
gh issue edit 123 -R benchmark-action/github-action-benchmark --title "New title"
43+
gh issue edit 123 -R benchmark-action/github-action-benchmark --body "Updated body"
44+
gh issue edit 123 -R benchmark-action/github-action-benchmark --add-label "priority:high" --remove-label "triage"
45+
gh issue edit 123 -R benchmark-action/github-action-benchmark --add-assignee "@me"
46+
```
47+
48+
### Close / Reopen
49+
```bash
50+
gh issue close 123 -R benchmark-action/github-action-benchmark
51+
gh issue close 123 -R benchmark-action/github-action-benchmark --reason "not planned"
52+
gh issue reopen 123 -R benchmark-action/github-action-benchmark
53+
```
54+
55+
### Comment on an issue
56+
```bash
57+
gh issue comment 123 -R benchmark-action/github-action-benchmark --body "My comment"
58+
```
59+
60+
### Pin / Unpin
61+
```bash
62+
gh issue pin 123 -R benchmark-action/github-action-benchmark
63+
gh issue unpin 123 -R benchmark-action/github-action-benchmark
64+
```
65+
66+
## Workflow
67+
68+
1. **Determine the intent** from the user's request (list, view, create, edit, close, comment, etc.)
69+
2. **Always pass** `-R benchmark-action/github-action-benchmark` unless the user says otherwise
70+
3. **Run the appropriate `gh issue` command** using the Bash tool
71+
4. **Present the output** clearly; for `--json` output, summarize the relevant fields rather than dumping raw JSON
72+
73+
## Tips
74+
75+
- Issue numbers and URLs are both valid arguments
76+
- Use `--json fields` + `--jq expression` for precise filtering
77+
- `gh issue status -R benchmark-action/github-action-benchmark` shows issues relevant to you

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,20 @@ which means there is no limit.
532532

533533
If set to `true`, the workflow will skip fetching branch defined with the `gh-pages-branch` variable.
534534

535+
#### `go-force-package-suffix` (Optional)
536+
537+
- Type: Boolean
538+
- Default: `false`
539+
540+
Go-specific option. When running benchmarks across multiple packages, this action automatically appends
541+
the package name as a suffix to benchmark names to disambiguate them (e.g., `BenchmarkFoo (github.com/example/pkg1)`).
542+
By default, if the benchmark name already contains a reference to the package path, the suffix is skipped
543+
to avoid duplication.
544+
545+
If set to `true`, the package suffix is always added regardless of whether the benchmark name already
546+
contains a package reference. This can be useful when you want consistent naming or when the automatic
547+
detection doesn't match your naming conventions.
548+
535549

536550
### Action outputs
537551

action-types.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,5 @@ inputs:
5555
type: string
5656
max-items-in-chart:
5757
type: integer
58+
go-force-package-suffix:
59+
type: boolean

action.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ inputs:
7979
max-items-in-chart:
8080
description: 'Max data points in a benchmark chart to avoid making the chart too busy. Value must be unsigned integer. No limit by default'
8181
required: false
82+
go-force-package-suffix:
83+
description: 'Force adding package suffix to Go benchmark names even when name already contains package reference'
84+
required: false
85+
default: 'false'
8286

8387
runs:
8488
using: 'node20'

src/config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export interface Config {
2525
externalDataJsonPath: string | undefined;
2626
maxItemsInChart: number | null;
2727
ref: string | undefined;
28+
goForcePackageSuffix: boolean;
2829
}
2930

3031
export const VALID_TOOLS = [
@@ -240,6 +241,7 @@ export async function configFromJobInput(): Promise<Config> {
240241
let externalDataJsonPath: undefined | string = core.getInput('external-data-json-path');
241242
const maxItemsInChart = getUintInput('max-items-in-chart');
242243
let failThreshold = getPercentageInput('fail-threshold');
244+
const goForcePackageSuffix = getBoolInput('go-force-package-suffix');
243245

244246
validateToolType(tool);
245247
outputFilePath = await validateOutputFilePath(outputFilePath);
@@ -287,5 +289,6 @@ export async function configFromJobInput(): Promise<Config> {
287289
maxItemsInChart,
288290
failThreshold,
289291
ref,
292+
goForcePackageSuffix,
290293
};
291294
}

src/extract.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,23 @@ function extractCargoResult(output: string): BenchmarkResult[] {
343343
return ret;
344344
}
345345

346-
export function extractGoResult(output: string): BenchmarkResult[] {
346+
function containsPackageRef(name: string, pkg: string): boolean {
347+
const segments = pkg.split('/');
348+
// Require at least 2 segments to avoid false positives (e.g., "cache" appearing in BenchmarkCache)
349+
const minSegments = 2;
350+
for (let i = 0; i <= segments.length - minSegments; i++) {
351+
const suffix = segments.slice(i).join('/');
352+
if (name.includes(suffix)) return true;
353+
if (name.includes(suffix.replace(/\//g, '_'))) return true;
354+
}
355+
return false;
356+
}
357+
358+
export interface GoExtractOptions {
359+
forcePackageSuffix?: boolean;
360+
}
361+
362+
export function extractGoResult(output: string, options: GoExtractOptions = {}): BenchmarkResult[] {
347363
// Split into sections by "pkg:" lines, keeping package name with each section
348364
const sections = output.split(/^pkg:\s+/m).map((section, index) => {
349365
if (index === 0) return { pkg: '', lines: section.split(/\r?\n/g) };
@@ -386,7 +402,9 @@ export function extractGoResult(output: string): BenchmarkResult[] {
386402
pieces.unshift(pieces[0], remainder.slice(remainder.indexOf(pieces[1])));
387403
}
388404

389-
const baseName = hasMultiplePackages && pkg ? `${name} (${pkg})` : name;
405+
const shouldAddPackageSuffix =
406+
hasMultiplePackages && pkg && (options.forcePackageSuffix || !containsPackageRef(name, pkg));
407+
const baseName = shouldAddPackageSuffix ? `${name} (${pkg})` : name;
390408
// Chunk into [value, unit] pairs and map to results
391409
return chunkPairs(pieces).map(([valueStr, unit], i) => ({
392410
name: i > 0 ? `${baseName} - ${unit}` : baseName,
@@ -708,7 +726,7 @@ export async function extractResult(config: Config): Promise<Benchmark> {
708726
benches = extractCargoResult(output);
709727
break;
710728
case 'go':
711-
benches = extractGoResult(output);
729+
benches = extractGoResult(output, { forcePackageSuffix: config.goForcePackageSuffix });
712730
break;
713731
case 'benchmarkjs':
714732
benches = extractBenchmarkJsResult(output);

test/config.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,4 +371,32 @@ describe('configFromJobInput()', function () {
371371
A.ok(path.isAbsolute(config.benchmarkDataDirPath), config.benchmarkDataDirPath);
372372
A.equal(config.benchmarkDataDirPath, path.join(absCwd, 'outdir'));
373373
});
374+
375+
describe('go-force-package-suffix', () => {
376+
it('parses go-force-package-suffix as true', async () => {
377+
mockInputs({ ...defaultInputs, 'go-force-package-suffix': 'true' });
378+
const config = await configFromJobInput();
379+
A.equal(config.goForcePackageSuffix, true);
380+
});
381+
382+
it('parses go-force-package-suffix as false', async () => {
383+
mockInputs({ ...defaultInputs, 'go-force-package-suffix': 'false' });
384+
const config = await configFromJobInput();
385+
A.equal(config.goForcePackageSuffix, false);
386+
});
387+
388+
it('defaults go-force-package-suffix to false when not set', async () => {
389+
mockInputs({ ...defaultInputs });
390+
const config = await configFromJobInput();
391+
A.equal(config.goForcePackageSuffix, false);
392+
});
393+
394+
it('throws on invalid go-force-package-suffix value', async () => {
395+
mockInputs({ ...defaultInputs, 'go-force-package-suffix': 'invalid' });
396+
await A.rejects(
397+
configFromJobInput,
398+
/'go-force-package-suffix' input must be boolean value 'true' or 'false' but got 'invalid'/,
399+
);
400+
});
401+
});
374402
});

test/extractGoResult.spec.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,4 +287,106 @@ describe('extractGoResult()', () => {
287287
expect(results[3].name).toBe('BenchmarkFoo (github.com/example/pkg2)');
288288
});
289289
});
290+
291+
describe('avoiding duplicate package suffix', () => {
292+
it('skips suffix when name contains full package path', () => {
293+
const output = dedent`
294+
pkg: github.com/example/pkg1
295+
BenchmarkFoo_github.com/example/pkg1-8 5000000 100 ns/op
296+
pkg: github.com/example/pkg2
297+
BenchmarkBar-8 3000000 200 ns/op
298+
`;
299+
const results = extractGoResult(output);
300+
expect(results[0].name).toBe('BenchmarkFoo_github.com/example/pkg1');
301+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
302+
});
303+
304+
it('skips suffix when name contains normalized full package path', () => {
305+
const output = dedent`
306+
pkg: github.com/example/pkg1
307+
BenchmarkFoo_github.com_example_pkg1-8 5000000 100 ns/op
308+
pkg: github.com/example/pkg2
309+
BenchmarkBar-8 3000000 200 ns/op
310+
`;
311+
const results = extractGoResult(output);
312+
expect(results[0].name).toBe('BenchmarkFoo_github.com_example_pkg1');
313+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
314+
});
315+
316+
it('skips suffix when name contains last package segments', () => {
317+
const output = dedent`
318+
pkg: github.com/example/pkg1
319+
BenchmarkFoo_example_pkg1-8 5000000 100 ns/op
320+
pkg: github.com/example/pkg2
321+
BenchmarkBar-8 3000000 200 ns/op
322+
`;
323+
const results = extractGoResult(output);
324+
expect(results[0].name).toBe('BenchmarkFoo_example_pkg1');
325+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
326+
});
327+
328+
it('skips suffix for gofiber-style workaround', () => {
329+
const output = dedent`
330+
pkg: github.com/gofiber/fiber/v3/middleware/cache
331+
BenchmarkAppendMsgitem_middleware_cache-8 5000000 100 ns/op
332+
pkg: github.com/gofiber/fiber/v3/middleware/csrf
333+
BenchmarkAppendMsgitem-8 3000000 200 ns/op
334+
`;
335+
const results = extractGoResult(output);
336+
expect(results[0].name).toBe('BenchmarkAppendMsgitem_middleware_cache');
337+
expect(results[1].name).toBe('BenchmarkAppendMsgitem (github.com/gofiber/fiber/v3/middleware/csrf)');
338+
});
339+
340+
it('still adds suffix when only single segment matches (avoid false positives)', () => {
341+
const output = dedent`
342+
pkg: github.com/example/cache
343+
BenchmarkCache-8 5000000 100 ns/op
344+
pkg: github.com/example/store
345+
BenchmarkStore-8 3000000 200 ns/op
346+
`;
347+
const results = extractGoResult(output);
348+
// "cache" appears in name but it's not a deliberate suffix - still add package
349+
expect(results[0].name).toBe('BenchmarkCache (github.com/example/cache)');
350+
expect(results[1].name).toBe('BenchmarkStore (github.com/example/store)');
351+
});
352+
});
353+
354+
describe('forcePackageSuffix option', () => {
355+
it('forces package suffix when forcePackageSuffix is true', () => {
356+
const output = dedent`
357+
pkg: github.com/example/pkg1
358+
BenchmarkFoo_example_pkg1-8 5000000 100 ns/op
359+
pkg: github.com/example/pkg2
360+
BenchmarkBar-8 3000000 200 ns/op
361+
`;
362+
const results = extractGoResult(output, { forcePackageSuffix: true });
363+
expect(results[0].name).toBe('BenchmarkFoo_example_pkg1 (github.com/example/pkg1)');
364+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
365+
});
366+
367+
it('skips suffix by default when name contains package ref', () => {
368+
const output = dedent`
369+
pkg: github.com/example/pkg1
370+
BenchmarkFoo_example_pkg1-8 5000000 100 ns/op
371+
pkg: github.com/example/pkg2
372+
BenchmarkBar-8 3000000 200 ns/op
373+
`;
374+
// No options passed - should use default behavior
375+
const results = extractGoResult(output);
376+
expect(results[0].name).toBe('BenchmarkFoo_example_pkg1');
377+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
378+
});
379+
380+
it('skips suffix when forcePackageSuffix is explicitly false', () => {
381+
const output = dedent`
382+
pkg: github.com/example/pkg1
383+
BenchmarkFoo_example_pkg1-8 5000000 100 ns/op
384+
pkg: github.com/example/pkg2
385+
BenchmarkBar-8 3000000 200 ns/op
386+
`;
387+
const results = extractGoResult(output, { forcePackageSuffix: false });
388+
expect(results[0].name).toBe('BenchmarkFoo_example_pkg1');
389+
expect(results[1].name).toBe('BenchmarkBar (github.com/example/pkg2)');
390+
});
391+
});
290392
});

test/write.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
188188
maxItemsInChart: null,
189189
failThreshold: 2.0,
190190
ref: undefined,
191+
goForcePackageSuffix: false,
191192
};
192193

193194
const savedRepository = {
@@ -1006,6 +1007,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
10061007
maxItemsInChart: null,
10071008
failThreshold: 2.0,
10081009
ref: undefined,
1010+
goForcePackageSuffix: false,
10091011
};
10101012

10111013
function gitHistory(

0 commit comments

Comments
 (0)