Conversation
…alysis Add optional jobs for integration testing and test determinism verification, enabling callers to consolidate CI workflows. New inputs: - enable_integration_tests (boolean, default false) - integration_test_command (string, default 'make test-integration') - enable_test_determinism (boolean, default false) - test_determinism_runs (number, default 3)
WalkthroughAdds workflow inputs to enable integration and determinism testing, two conditional jobs ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/go-pr-analysis.yml (1)
7-86:⚠️ Potential issue | 🟠 MajorAdd
workflow_dispatchanddry_runbefore expanding this public API.This reusable workflow still exposes only
workflow_call, and there is nodry_runboolean input. That prevents manual validation of the new jobs and violates the repository contract for shared reusable workflows. Based on learnings: Enforce that every reusable GitHub Actions workflow in LerianStudio/github-actions-shared-workflows exposes both workflow_call and workflow_dispatch triggers, and includes a dry_run input (default: false).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/go-pr-analysis.yml around lines 7 - 86, The workflow only exposes workflow_call and lacks a dry_run input and a workflow_dispatch trigger; add a top-level workflow_dispatch trigger alongside workflow_call and add a new boolean input dry_run (default: false) in the inputs block (same level as runner_type, enable_tests, etc.) so callers can manually dispatch and opt into dry-run behavior; ensure the new dry_run input name is exactly dry_run and default is false to match the repo contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/go-pr-analysis.yml:
- Around line 850-861: The determinism step currently runs "go test -count=1
-shuffle=on ./..." directly which bypasses the same Makefile targets and package
exclusions used by the main "tests" job; update this step (the one named "Run
tests ${{ inputs.test_determinism_runs }} times") to invoke the exact same test
invocation as the "tests" job (e.g., call the Makefile test target or reuse the
same command/step output used by the tests job) instead of raw "go test ./...",
so the determinism loop runs the identical suite (honoring Makefile targets and
excluding /tests/ and /api/) each iteration.
---
Outside diff comments:
In @.github/workflows/go-pr-analysis.yml:
- Around line 7-86: The workflow only exposes workflow_call and lacks a dry_run
input and a workflow_dispatch trigger; add a top-level workflow_dispatch trigger
alongside workflow_call and add a new boolean input dry_run (default: false) in
the inputs block (same level as runner_type, enable_tests, etc.) so callers can
manually dispatch and opt into dry-run behavior; ensure the new dry_run input
name is exactly dry_run and default is false to match the repo contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6ee6317-ff65-4469-ba90-e894b8d7aa0a
📒 Files selected for processing (1)
.github/workflows/go-pr-analysis.yml
Detect Makefile test target and exclude /tests/ and /api/ packages to match the same suite executed by the tests job.
bedatty
dismissed
coderabbitai[bot]’s stale review
All comments addressed. The determinism suite alignment fix was applied in bc72e23. The workflow_dispatch/dry_run suggestion is out of scope for this PR — it's a pre-existing gap unrelated to the new jobs.
There was a problem hiding this comment.
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 @.github/workflows/go-pr-analysis.yml:
- Around line 870-878: When steps.detect-make.outputs.use_make == "true" the
workflow sets TEST_CMD="make test" which does not include the determinism flags;
change the logic so TEST_CMD always runs go test with -count=1 -shuffle=on (or,
if you must keep Makefile, set an env var/MAKEFLAGS the Makefile honors and
append -count=1 -shuffle=on) by replacing TEST_CMD="make test" with a command
that invokes go test on the same PACKAGES variable with the determinism flags
(reference: TEST_CMD, PACKAGES, steps.detect-make.outputs.use_make).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2359986e-92d9-448d-8b6d-f6e83e95d6d6
📒 Files selected for processing (3)
.github/dependabot.yml.github/workflows/go-ci.yml.github/workflows/go-pr-analysis.yml
This reverts commit 6d22810.
Remove Makefile detection from test-determinism job. The tests job already validates that tests pass via Makefile; this job must always use go test with -count=1 -shuffle=on to actually verify determinism.
bedatty
dismissed
coderabbitai[bot]’s stale review
All comments addressed in commit c0f990d.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/go-pr-analysis.yml:
- Line 897: The generated failed_jobs string in the workflow uses inline
conditional concatenation and can produce a trailing comma (e.g., "Integration
Tests, "); update the construction of failed_jobs (the failed_jobs expression in
the slack-notify step) to build an array-like list and then join/trim separators
(or trim trailing commas/spaces) before emitting the final string so no trailing
comma remains (alternatively restructure to emit a JSON array and join it in the
notification step); locate the failed_jobs expression in the workflow and
replace its concatenation logic with a trimmed/joined result.
- Around line 71-86: Add a reusable-workflow docs file named go-pr-analysis.md
under docs that includes the Lerian branding header and documents all inputs,
outputs and usage examples; specifically document the new inputs
enable_integration_tests, integration_test_command, enable_test_determinism and
test_determinism_runs (their types, defaults and purpose), list any outputs the
workflow emits, and include a short example snippet showing how to call the
reusable workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7b01f3e-87dc-452e-aaee-cccfba297cbb
📒 Files selected for processing (1)
.github/workflows/go-pr-analysis.yml
bedatty
dismissed
coderabbitai[bot]’s stale review
All comments addressed — docs updated in 772859c, trailing comma nitpick declined (pre-existing repo-wide pattern).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/go-pr-analysis-workflow.md (1)
1-1:⚠️ Potential issue | 🟡 MinorFilename doesn't match workflow file naming convention.
Per coding guidelines, the documentation filename must exactly match the workflow file. Since the workflow is
go-pr-analysis.yml, this file should bego-pr-analysis.md, notgo-pr-analysis-workflow.md.As per coding guidelines: "Workflow documentation. Filename must exactly match the workflow file (e.g.,
go-ci.yml→docs/go-ci.md)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/go-pr-analysis-workflow.md` at line 1, The docs filename must match the workflow file name exactly; rename the documentation file currently referenced as "go-pr-analysis-workflow.md" to "go-pr-analysis.md" so it pairs with the workflow "go-pr-analysis.yml" (update any references if present), ensuring the docs filename equals the workflow filename without the .yml extension.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/go-pr-analysis-workflow.md`:
- Around line 167-172: Insert a blank line before the "### integration-tests"
and "### test-determinism" headings to match the document's section spacing and
resolve MD022; update the lines that define those job section headings (look for
the strings "### integration-tests" and "### test-determinism") so each is
preceded by an empty line.
---
Outside diff comments:
In `@docs/go-pr-analysis-workflow.md`:
- Line 1: The docs filename must match the workflow file name exactly; rename
the documentation file currently referenced as "go-pr-analysis-workflow.md" to
"go-pr-analysis.md" so it pairs with the workflow "go-pr-analysis.yml" (update
any references if present), ensuring the docs filename equals the workflow
filename without the .yml extension.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0de8ec3d-9481-45ff-aec2-33baf9a6c802
📒 Files selected for processing (1)
docs/go-pr-analysis-workflow.md
bedatty
dismissed
coderabbitai[bot]’s stale review
Comment declined — format matches existing document pattern.
github-actions
bot
deleted the
feat/go-pr-analysis-integration-determinism
branch
1 participant
GitHub Actions Shared Workflows
Description
Add optional integration tests and test determinism jobs to the
go-pr-analysisreusable workflow. This allows caller repos to consolidate their CI workflows instead of maintaining separatecode-quality.ymlfiles.New inputs:
enable_integration_tests(boolean, defaultfalse) — runs integration tests via configurable commandintegration_test_command(string, defaultmake test-integration) — command to executeenable_test_determinism(boolean, defaultfalse) — runs tests multiple times with-shuffle=ontest_determinism_runs(number, default3) — number of determinism runsBoth jobs are monorepo-aware (matrix per app), support private Go modules, and are included in Slack notifications.
Type of Change
feat: New workflow or new input/output/step in an existing workflowBreaking Changes
None. All new inputs default to
false/disabled — existing callers are unaffected.Testing
@developor the beta tagCaller repo / workflow run: Will validate via tracer repo after merge to develop.
Related Issues
Summary by CodeRabbit
Chores
Documentation