-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add e2e.yml #83
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.
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
=======================================
Coverage 94.48% 94.48%
=======================================
Files 3 3
Lines 145 145
=======================================
Hits 137 137
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
""" WalkthroughA new GitHub Actions workflow named "E2E Test" has been introduced. This workflow automates cloning a target repository, replacing its pre-commit configuration with a minimal clang-format hook config, installing pre-commit, and running all pre-commit hooks on the repository files, supporting both default and user-supplied options. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Target Repo
participant cpp-linter-hooks
User->>GitHub Actions: Push/PR to main or manual dispatch
GitHub Actions->>GitHub Actions: Checkout current repo
GitHub Actions->>GitHub Actions: Setup Python 3.13 and pip
GitHub Actions->>GitHub Actions: Install pre-commit
GitHub Actions->>Target Repo: Clone (default: llvm/llvm-project)
GitHub Actions->>Target Repo: Replace .pre-commit-config.yaml with clang-format hook config
GitHub Actions->>Target Repo: Install pre-commit hooks
GitHub Actions->>Target Repo: Run pre-commit on all files
Target Repo->>cpp-linter-hooks: Use clang-format hook (v20.1.7)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/e2e.yml (1)
37-45
: Ensure requested Clang binary is actually availableThe runner doesn’t ship with
clang-format-20
.
Without installing the matching tool, the hook will fail (or default to an older clang). Add an explicit install step before running pre-commit:- name: Install dependencies run: | python -m pip install --upgrade pip python -m pip install pre-commit + + - name: Install clang-format + run: | + sudo apt-get update -qq + sudo apt-get install -y clang-format-${{ github.event.inputs.clang_version || '20' }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/e2e.yml
33-33: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run benchmarks
- GitHub Check: Test cpp-linter-hooks
🔇 Additional comments (1)
.github/workflows/e2e.yml (1)
32-35
: Update to the current major ofsetup-python
actions/setup-python@v4
is already flagged by actionlint as too old.
GitHub released v5 with improved caching and deprecation notices – migrating is trivial and future-proofs the workflow.- - name: Set up Python 3.11 - uses: actions/setup-python@v4 + - name: Set up Python 3.11 + uses: actions/setup-python@v5
CodSpeed Performance ReportMerging #83 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/e2e.yml (1)
53-55
:--version
still turnsclang-format
into a no-op
Passing--version
prints the version and exits, so the hook never formats any files and the test becomes meaningless. This was flagged previously and is still present.- - id: clang-format - args: [--style=${{ github.event.inputs.style || 'Google' }}, --version=${{ github.event.inputs.clang_version || '20' }}] + - id: clang-format + args: [--style=${{ github.event.inputs.style || 'Google' }}]
🧹 Nitpick comments (1)
.github/workflows/e2e.yml (1)
60-63
:pre-commit install
is unnecessary in CI
pre-commit run --all-files
does not require the git hook to be installed. Dropping theinstall
call shaves ~2 s and avoids side effects.- pre-commit install pre-commit run --all-files
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/e2e.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e-test
.github/workflows/e2e.yml
Outdated
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
python -m pip install pre-commit | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install the required clang-format
binary
The workflow relies on clang-format
(potentially version 20) being present on the runner, but nothing installs it. Without the binary every run will fail. Add an explicit install step before running pre-commit.
- name: Install dependencies
run: |
+ sudo apt-get update
+ sudo apt-get install -y clang-format-${{ github.event.inputs.clang_version || '20' }}
python -m pip install --upgrade pip
python -m pip install pre-commit
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Install dependencies | |
run: | | |
python -m pip install --upgrade pip | |
python -m pip install pre-commit | |
- name: Install dependencies | |
run: | | |
sudo apt-get update | |
sudo apt-get install -y clang-format-${{ github.event.inputs.clang_version || '20' }} | |
python -m pip install --upgrade pip | |
python -m pip install pre-commit |
🤖 Prompt for AI Agents
In .github/workflows/e2e.yml around lines 35 to 39, the workflow installs pip
and pre-commit but does not install the required clang-format binary, causing
runs to fail. Add a step before installing pre-commit to explicitly install
clang-format on the runner, ensuring the correct version is available for
pre-commit hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/e2e.yml (2)
55-59
:clang-format
never runs with--version
– remove the flag again
We previously discussed that passing--version
causesclang-format
to exit immediately, so the hook reports success without touching any files. The flag has resurfaced here and will break every test run.- - id: clang-format - args: [ - --style=${{ github.event.inputs.style || 'file' }}, - --version=${{ matrix.clang_version }} - ] + - id: clang-format + args: [ + --style=${{ github.event.inputs.style || 'file' }} + ]
37-41
:clang-format
binary is missing – workflow will error out
The job installspre-commit
but never installs the specificclang-format-<version>
binaries required by the hook. All eight matrix legs will fail with “clang-format: command not found”.Add an explicit install step before running the hooks:
- name: Install Python dependencies run: | python -m pip install --upgrade pip python -m pip install pre-commit + + - name: Install clang-format ${{ matrix.clang_version }} + run: | + sudo apt-get update -y + sudo apt-get install -y clang-format-${{ matrix.clang_version }} || sudo apt-get install -y clang-format
🧹 Nitpick comments (1)
.github/workflows/e2e.yml (1)
44-45
: Use the already-defined default instead of the||
fallback
workflow_dispatch
inputs already supply a default value. The extra|| 'llvm/llvm-project'
makes the expression harder to read and, onpush
/pull_request
events wheregithub.event.inputs
is undefined, resolves tonull
, producing a URL ending in.git
. Prefer the shorter and always-defined form:- git clone --depth=1 https://github.com/${{ github.event.inputs.target_repo || 'llvm/llvm-project' }}.git test-repo + git clone --depth=1 https://github.com/${{ inputs.target_repo }}.git test-repo
|
closes #82
Summary by CodeRabbit