Skip to content

feat(ci): automate and enhance performance validation, add semantic PR checks, and consolidate build steps #562

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

Merged
rjmurillo merged 41 commits into from
Jul 19, 2025
Merged

feat(ci): automate and enhance performance validation, add semantic PR checks, and consolidate build steps #562

Changes from 25 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
394c9a6
ci: run performance validation nightly and increase benchmark iterati…
rjmurillo Jul 19, 2025
2a59ceb
ci: run build, test, and performance validation on both windows-11-ar…
rjmurillo Jul 19, 2025
fdeee68
ci: deduplicate build and performance steps using composite action
rjmurillo Jul 19, 2025
1993586
ci: rename composite action to setup-restore-build for clarity
rjmurillo Jul 19, 2025
f718940
ci: fix artifact naming for performance job to prevent OS conflicts
rjmurillo Jul 19, 2025
a800e57
ci: run performance validation nightly and increase benchmark iterati…
rjmurillo Jul 19, 2025
42a53cb
ci: run build, test, and performance validation on both windows-11-ar…
rjmurillo Jul 19, 2025
d66ffe3
ci: deduplicate build and performance steps using composite action
rjmurillo Jul 19, 2025
752aee8
ci: rename composite action to setup-restore-build for clarity
rjmurillo Jul 19, 2025
60c827e
ci: fix artifact naming for performance job to prevent OS conflicts
rjmurillo Jul 19, 2025
c3576b7
Update main.yml reusable step syntax
rjmurillo Jul 19, 2025
822281c
Update main.yml step reuse syntax
rjmurillo Jul 19, 2025
7527716
ci: add semantic PR validation check (#555)
rjmurillo Jul 19, 2025
a871c51
Update main.yml reusable step syntax to avoid path problem
rjmurillo Jul 19, 2025
eac1439
Update main.yml
rjmurillo Jul 19, 2025
6253322
Add Dependency Review Action and improve Moq detection
rjmurillo Jul 19, 2025
3539ab4
Update workflows to include checkout and build steps
rjmurillo Jul 19, 2025
a3e37d6
fix: specify shell for NuGet restore and build steps in action.yml
rjmurillo Jul 19, 2025
fa85571
fix: expand OS matrix for perf jobs to include latest versions and macOS
rjmurillo Jul 19, 2025
5cb4e86
ci: enhance perf job with caching, regression gates, and actionable s…
rjmurillo Jul 19, 2025
8daaad0
fix: remove unnecessary performance summary file from upload path
rjmurillo Jul 19, 2025
8459cf1
ci: trigger perf job only on relevant file or dependency changes
rjmurillo Jul 19, 2025
523d94c
ci: enable ad hoc performance job via workflow_dispatch input
rjmurillo Jul 19, 2025
4bd18e5
ci: remove unnecessary path filters from push trigger
rjmurillo Jul 19, 2025
ce8a588
Update actions and refine job conditions
rjmurillo Jul 19, 2025
90bbf90
fix: update performance summary publishing to handle multiple report …
rjmurillo Jul 19, 2025
24d4820
fix: rename performance files artifact for consistency
rjmurillo Jul 19, 2025
9861437
feat: add publishing of baseline performance summaries to GitHub
rjmurillo Jul 19, 2025
5252413
fix: remove caching of baseline performance file
rjmurillo Jul 19, 2025
2a27ab2
feat: implement cached baseline performance caching and comparison logic
rjmurillo Jul 19, 2025
cc3153d
feat: add option to force baseline benchmarks in performance workflow
rjmurillo Jul 19, 2025
23500e0
fix: update baseline performance cache handling in workflow
rjmurillo Jul 19, 2025
490895c
fix: update temporary directory path to use artifacts folder
rjmurillo Jul 19, 2025
b908b5c
fix: improve warning message for cached baseline results and streamli…
rjmurillo Jul 19, 2025
6b57b5e
feat: add support for using cached baseline results in performance sc…
rjmurillo Jul 19, 2025
6a50f71
fix: correct variable name for command arguments in performance test …
rjmurillo Jul 19, 2025
d191243
feat: add CI performance benchmarking and baseline caching documentation
rjmurillo Jul 19, 2025
3ca4209
docs: document CI performance benchmarking and baseline caching
rjmurillo Jul 19, 2025
46cc428
fix: correct syntax for performance artifact naming in workflow
rjmurillo Jul 19, 2025
5cd10c4
fix: ensure baseline performance cache upload only occurs on successf…
rjmurillo Jul 19, 2025
3023beb
fix: fail early when the expected perf baseline file is not found
rjmurillo Jul 19, 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
32 changes: 29 additions & 3 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
name: Main build

on:
workflow_dispatch: # Allow running the workflow manually from the GitHub UI
workflow_dispatch:
inputs:
run_performance:
description: 'Run performance job? (true/false)'
required: false
default: false
type: boolean
pull_request:
branches:
- main
Expand Down Expand Up @@ -110,7 +116,10 @@ jobs:
if-no-files-found: error

performance:
if: github.event_name == 'schedule' && github.ref == 'refs/heads/main'
# Run if manually dispatched (with input), on schedule, or if explicitly requested
if: |
(github.event_name == 'schedule' && github.ref == 'refs/heads/main') ||
(github.event_name == 'workflow_dispatch' && github.event.inputs.run_performance == 'true')
Comment thread
rjmurillo marked this conversation as resolved.
Comment on lines +124 to +127

@coderabbitai coderabbitai Bot Jul 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

performance job never runs on PRs – contradicts PR objectives

The condition omits the pull_request event, yet the PR description states “triggered on pull requests”.
Add the PR path to avoid silently skipping performance validation for every PR.

-      (github.event_name == 'workflow_dispatch' && github.event.inputs.run_performance == 'true')
+      (github.event_name == 'workflow_dispatch' && github.event.inputs.run_performance == 'true') ||
+      (github.event_name == 'pull_request')
🤖 Prompt for AI Agents 
In .github/workflows/main.yml around lines 124 to 127, the condition for running
the performance job excludes the pull_request event, which contradicts the PR
description stating it should trigger on pull requests. Modify the if condition
to also include the pull_request event so that the performance job runs on PRs
as intended, ensuring performance validation is not skipped during pull
requests.

strategy:
fail-fast: false
matrix:
Expand All @@ -123,6 +132,14 @@ jobs:
with:
fetch-depth: 0

- name: Cache baseline performance results
uses: actions/cache@v4
with:
path: ./artifacts/performance/baseline.json
key: baseline-performance-${{ runner.os }}-${{ github.ref }}
restore-keys: |
baseline-performance-${{ runner.os }}-

Comment on lines +153 to +159

@coderabbitai coderabbitai Bot Jul 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Cache key risks hash-collision when baseline SHA changes across branches

The cache is keyed on ${{ steps.get-baseline-sha.outputs.sha }} only; if two different PRs reference the same baseline commit but diverge in BenchmarkDotNet parameters, stale artefacts might be used.
Add the BDN parameter hash (or workflow revision) to the key to ensure correctness.

🤖 Prompt for AI Agents 
In .github/workflows/main.yml around lines 150 to 156, the cache key for the
baseline performance cache only uses the baseline SHA, which risks hash
collisions when different PRs share the same baseline commit but have different
BenchmarkDotNet parameters. To fix this, modify the cache key to include a hash
or identifier of the BenchmarkDotNet parameters or the workflow revision,
ensuring the cache key uniquely reflects both the baseline SHA and the
parameters to prevent stale artifact usage.

@rjmurillo rjmurillo Jul 19, 2025

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@coderabbitai create an issue to track this suggestion

@coderabbitai coderabbitai Bot Jul 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

I'll create an issue to track this cache key collision suggestion.

@rjmurillo I've created an issue to track the cache key collision suggestion. The issue will help ensure this improvement is properly prioritized and implemented in a future PR.

🏁 Script executed:

#!/bin/bash
# Create GitHub issue to track the cache key collision suggestion
gh issue create \
  --title "Performance cache key collision risk with shared baseline SHA across different BenchmarkDotNet parameters" \
  --body "## Problem

The performance workflow cache key in \`.github/workflows/main.yml\` currently only uses the baseline SHA (\`baseline-performance-\${{ matrix.os }}-\${{ steps.get-baseline-sha.outputs.sha }}\`), which creates a risk of hash collisions when different PRs reference the same baseline commit but have different BenchmarkDotNet parameters.

## Impact

If two different PRs share the same baseline commit but diverge in BenchmarkDotNet parameters, stale artifacts might be used, leading to incorrect performance comparisons.

## Suggested Solution

Modify the cache key to include a hash or identifier of the BenchmarkDotNet parameters or the workflow revision, ensuring the cache key uniquely reflects both the baseline SHA and the parameters.

For example:
\`\`\`
key: baseline-performance-\${{ matrix.os }}-\${{ steps.get-baseline-sha.outputs.sha }}-\${{ hashFiles('build/scripts/perf/**') }}
\`\`\`

## References

- PR: https://github.com/rjmurillo/moq.analyzers/pull/562
- Comment: https://github.com/rjmurillo/moq.analyzers/pull/562#discussion_r2217497278
- Requested by: @rjmurillo" \
  --assignee rjmurillo

Length of output: 1305

@rjmurillo Perfect! I've successfully created issue #564 to track the cache key collision risk. The issue includes:

  • A clear description of the problem with the current cache key implementation
  • The potential impact of hash collisions when different PRs share baseline commits
  • A suggested solution using hashFiles() to include BenchmarkDotNet parameter changes
  • Proper backlinks to this PR and comment thread
  • Assignment to you for future prioritization

The issue is now available at: #564

🧠 Learnings used 
Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.110Z
Learning: If you disagree with a bot's suggestion, explain why in the PR description.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.110Z
Learning: Address all feedback from automated bots (e.g., Codeclimate, formatting/linting bots) as you would human reviewers; resolve all formatting and linting issues flagged by bots or CI before requesting review or merging.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T08:09:21.038Z
Learning: When making changes, follow this workflow: Edit/Add Code or Test → Run codacy_cli_analyze → Fix issues → Run/Update Tests → Update Docs → Commit & PR.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.110Z
Learning: Summarize all changes in the PR description and cite relevant lines from modified files for clarity.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T08:09:21.037Z
Learning: Applies to {README.md,docs/rules/README.md} : Update README.md and docs/rules/README.md if workflows or rules change.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T08:09:21.038Z
Learning: Cite relevant lines from modified files in PR summaries for clarity.

Learnt from: rjmurillo
PR: rjmurillo/moq.analyzers#325
File: .github/workflows/dependabot-approve-and-auto-merge.yml:8-8
Timestamp: 2025-01-22T00:47:24.622Z
Learning: For the moq.analyzers repository, ARM64 runner migration can proceed without staged rollout as there are minimal dependencies, with policy configurations to be adjusted post-merge.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.110Z
Learning: Every pull request must include evidence of running code formatting, building with warnings as errors, and running all unit tests (console log/text or screenshots) in the PR description.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T08:09:21.038Z
Learning: Every pull request must include evidence of steps 1–3 in the PR description (console log/text or screenshots). Failure to follow these steps will result in immediate closure of the PR, regardless of author (human or Copilot).

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-25T12:42:47.111Z
Learning: When making changes, follow the workflow: Edit/Add Code or Test → Run codacy_cli_analyze → Fix issues if any → Run/Update Tests → Fix test failures if any → Update Docs → Commit & PR.

Learnt from: CR
PR: rjmurillo/moq.analyzers#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T08:09:21.038Z
Learning: Keep commit messages short and in the imperative mood (e.g., "Add Moq1200 tests").

- name: Setup, Restore, and Build Solution
uses: ./.github/actions/setup-restore-build

Expand All @@ -134,9 +151,18 @@ jobs:
# These values balance CI cost and reliability, as recommended in issue #553.
run: ${{ github.workspace }}/build/scripts/perf/PerfCore.ps1 -v diag -diff -ci
Comment thread
rjmurillo marked this conversation as resolved.

- name: Publish performance summary to GitHub
if: success() || failure()
run: |
if (Test-Path artifacts/performance/perfResults/perfTest/SummaryGithub.md) {
Get-Content artifacts/performance/perfResults/perfTest/SummaryGithub.md | Out-File -Append -FilePath $env:GITHUB_STEP_SUMMARY
Comment thread
rjmurillo marked this conversation as resolved.
Outdated
}
shell: pwsh

- name: Upload performance files
uses: actions/upload-artifact@v4
if: success() || failure()
with:
name: Performance files (${{ matrix.os }})
path: "./artifacts/performance/**"
path: |
./artifacts/performance/**
Loading