Skip to content

fix: avoid duplicate package suffix in Go benchmarks [#336] - #337

Merged
ktrz merged 3 commits into
masterfrom
fix/336-avoid-duplicate-package-suffix
Mar 2, 2026
Merged

fix: avoid duplicate package suffix in Go benchmarks [#336] #337
ktrz merged 3 commits into
masterfrom
fix/336-avoid-duplicate-package-suffix

Conversation

@ktrz

@ktrzktrz commented Feb 24, 2026

Copy link
Copy Markdown
Member

resolves#336

  • Fixes duplicate suffixes for users who worked around Golang Benchmarks don't take Package Name into consideration #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
    detection

Summary by CodeRabbit

  • New Features

    • Added a Go option to force appending package path suffixes to benchmark names (with safeguards to avoid duplicate suffixes).
  • Documentation

    • README updated with the new Go configuration option.
    • New GitHub Issues Skill guide added for CLI-based issue workflows.
  • Tests

    • Added and extended tests covering package-suffix behavior and configuration parsing.

Users who worked around #264 by including package name in benchmark
names would get duplicate suffixes after #330. This detects when a
benchmark name already contains a reference to the package path
(full path, normalized with underscores, or ≥2 trailing segments)
and skips adding the suffix.
Add new action input to force package suffix on Go benchmark names
even when the name already contains a package reference. This allows
users to opt for consistent naming when the automatic duplicate
detection doesn't match their use case.
@ktrzktrz self-assigned this Feb 24, 2026
@coderabbitai

coderabbitaiBot commented Feb 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad1f40 and a54e7ac.

📒 Files selected for processing (1)
  • action-types.yml

📝 Walkthrough

Walkthrough

Adds a new boolean input go-force-package-suffix across action metadata and types, exposes it in runtime config, and updates Go benchmark extraction to conditionally append package suffixes (with a force option). Tests and README are updated; an unrelated skill doc was added.

Changes

Cohort / File(s)Summary
Action metadata & docs
action.yml, action-types.yml, README.md, .claude/skills/github-issues/SKILL.md
Declare new input go-force-package-suffix (boolean, default false) and document the option; added an unrelated GitHub Issues skill doc.
Configuration
src/config.ts
Add goForcePackageSuffix: boolean to exported Config; read value from input via getBoolInput('go-force-package-suffix').
Extraction logic
src/extract.ts
Introduce GoExtractOptions and containsPackageRef() helper; change extractGoResult() signature to accept options and conditionally append package suffix based on forcePackageSuffix or detection.
Tests
test/config.spec.ts, test/extractGoResult.spec.ts, test/write.spec.ts
Add config parsing tests for the new input; add many cases validating suffix detection, edge cases, Windows line endings, multi-package inputs, and force behavior; update test configs to include the new field.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • NachoVazquez

Poem

🐰 A suffix tale in code tonight,
Go names prance under test-run light.
If packages peek where names already be,
A clever check will set them free.
Force the flag, and let suffixes sing—hooray! 🎉

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe PR title clearly and concisely describes the main change: fixing duplicate package suffixes in Go benchmarks, directly addressing issue #336.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/336-avoid-duplicate-package-suffix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecovBot commented Feb 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.24%. Comparing base (7e8372b) to head (a54e7ac).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #337 +/- ##
==========================================
+ Coverage 89.13% 89.24% +0.11% 
==========================================
Files 16 16 Lines 948 958 +10 Branches 201 205 +4 ==========================================
+ Hits 845 855 +10 
Misses 103 103 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ktrz

ktrz commented Feb 24, 2026

Copy link
Copy Markdown
MemberAuthor

Hey @gaby, @ReneWerner87

Before I publish the new version, I wanted to make sure that it doesn't break your existing workaround by adding this new feature for including the package name.

There is a separate action input to force it to use the package names when possible, but if it finds the package name in the benchmark name, it won't do it. Chris, could you have a look at this PR and give me a green light that it should work as expected for gofiber?

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 535-547: The MD003 warning is because the new input section uses
ATX "#### `go-force-package-suffix` (Optional)" instead of the expected setext
style; change the heading for the `go-force-package-suffix` section to setext
style by replacing the ATX heading with the plain heading text
"go-force-package-suffix (Optional)" followed by an underline of '=' (or '-' for
subheading) on the next line so markdownlint MD003 passes, or alternatively
adjust the lint rule if setext is not desired.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e8372b and 0ad1f40.

📒 Files selected for processing (8)
  • .claude/skills/github-issues/SKILL.md
  • README.md
  • action.yml
  • src/config.ts
  • src/extract.ts
  • test/config.spec.ts
  • test/extractGoResult.spec.ts
  • test/write.spec.ts

Comment threadREADME.md
@ReneWerner87

Copy link
Copy Markdown

@ReneWerner87

Copy link
Copy Markdown

@ReneWerner87

Copy link
Copy Markdown

@ktrz

@ktrz

ktrz commented Feb 27, 2026

Copy link
Copy Markdown
MemberAuthor

Sorry, busy week. I'll get this in over the week.

@ktrz
ktrz merged commit 022dc9c into masterMar 2, 2026
29 checks passed
@ktrz
ktrz deleted the fix/336-avoid-duplicate-package-suffix branch March 2, 2026 21:28
414owen pushed a commit to dottxt-ai/github-action-benchmark that referenced this pull request Aug 20, 2026
…n#336] (benchmark-action#337)
- Fixes duplicate suffixes for users who worked around benchmark-action#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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go benchmarks: Avoid duplicating package name when workaround already applied

2 participants

@ktrz@ReneWerner87