-
-
Notifications
You must be signed in to change notification settings - Fork 18
chore: ci setup for pytests #118
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||||||||||||
| name: PR Tests & Coverage | ||||||||||||||||||||
|
|
||||||||||||||||||||
| on: | ||||||||||||||||||||
| pull_request: | ||||||||||||||||||||
| branches: | ||||||||||||||||||||
| - main | ||||||||||||||||||||
|
|
||||||||||||||||||||
| jobs: | ||||||||||||||||||||
| test: | ||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||
| permissions: | ||||||||||||||||||||
| contents: read | ||||||||||||||||||||
| pull-requests: write | ||||||||||||||||||||
|
Comment on lines
+11
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: Yes, GitHub restricts the GITHUB_TOKEN to read-only permissions for workflows triggered by pull requests from forked repositories by default, regardless of any write permissions explicitly set in the workflow YAML file [1][2][3]. This restriction is a security measure designed to prevent untrusted code (which controls the workflow file in the pull request) from gaining write access to the base repository [3]. When a pull request originates from a fork, any write permissions defined in the workflow are automatically adjusted to read-only [1][2][4]. There are two primary ways to bypass this restriction: 1. Repository Settings: An administrator can modify the repository's GitHub Actions settings to enable the option "Send write tokens to workflows from pull requests" [1][2][4]. This allows write tokens to be sent to workflows triggered by forked pull requests [1]. 2. Use pull_request_target: If the workflow is changed to use the pull_request_target event instead of the pull_request event, it will run with the base repository's context and permissions [2][3]. Because pull_request_target workflows execute code from the base repository's default branch rather than the untrusted fork, they are granted a read/write GITHUB_TOKEN and access to secrets [3]. Note that using pull_request_target requires careful security practices to prevent malicious code injection [3]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
file=".github/workflows/pr-checks.yml"
echo "=== file exists ==="
test -f "$file" && echo yes
echo "=== relevant lines ==="
nl -ba "$file" | sed -n '1,120p'
echo "=== search for workflow trigger and comment step ==="
rg -n "pull_request|pull_request_target|MishaKav/pytest-coverage-comment|permissions:|pull-requests: write|contents: read" "$file"Repository: StabilityNexus/MiniChain Length of output: 249 🏁 Script executed: #!/bin/bash
set -euo pipefail
file=".github/workflows/pr-checks.yml"
echo "=== file ==="
sed -n '1,120p' "$file"
echo "=== matches ==="
rg -n "pull_request|pull_request_target|MishaKav/pytest-coverage-comment|permissions:|pull-requests: write|contents: read" "$file"Repository: StabilityNexus/MiniChain Length of output: 1331 Fork PRs won’t be able to post this comment 🧰 Tools🪛 zizmor (1.26.1)[warning] 13-13: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) 🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| steps: | ||||||||||||||||||||
| - name: Checkout Code | ||||||||||||||||||||
| uses: actions/checkout@v4 | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win Pin third-party/GitHub actions to a commit SHA. zizmor flags Also applies to: 22-22, 38-38 🧰 Tools🪛 zizmor (1.26.1)[error] 17-17: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||
| with: | ||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||
|
Comment on lines
+16
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win Set This job never pushes, yet the default checkout leaves the GITHUB_TOKEN persisted in the local git config (flagged by zizmor as 🔒️ Proposed fix - name: Checkout Code
uses: actions/checkout@v4
with:
+ persist-credentials: false
fetch-depth: 0📝 Committable suggestion
Suggested change
🧰 Tools🪛 zizmor (1.26.1)[warning] 16-19: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) [error] 17-17: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||
|
|
||||||||||||||||||||
| - name: Setup Python | ||||||||||||||||||||
| uses: actions/setup-python@v5 | ||||||||||||||||||||
| with: | ||||||||||||||||||||
| python-version: "3.11" | ||||||||||||||||||||
| cache: "pip" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - name: Install Dependencies | ||||||||||||||||||||
| run: | | ||||||||||||||||||||
| python -m pip install --upgrade pip | ||||||||||||||||||||
| pip install -r requirements.txt | ||||||||||||||||||||
| pip install -r requirements-test.txt | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - name: Run Tests with Coverage | ||||||||||||||||||||
| run: | | ||||||||||||||||||||
| pytest tests/ -v --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=minichain | tee pytest-coverage.txt | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - name: Pytest coverage comment | ||||||||||||||||||||
| uses: MishaKav/pytest-coverage-comment@v1 | ||||||||||||||||||||
| with: | ||||||||||||||||||||
| pytest-coverage-path: ./pytest-coverage.txt | ||||||||||||||||||||
| junitxml-path: ./pytest.xml | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||||||||||||||||||
| name: Update Coverage Badge | ||||||||||||||||||||||
| on: | ||||||||||||||||||||||
| push: | ||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||
|
Comment on lines
+2
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win Add a concurrency group. Rapid successive pushes to ♻️ Proposed addition on:
push:
branches: [main]
+
+concurrency:
+ group: update-badge-${{ github.ref }}
+ cancel-in-progress: false📝 Committable suggestion
Suggested change
🧰 Tools🪛 zizmor (1.26.1)[warning] 2-4: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting (concurrency-limits) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||
| contents: write | ||||||||||||||||||||||
|
Comment on lines
+6
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win Scope Workflow-level permissions apply to every job in the file; scoping to the job that actually needs to commit follows least privilege and is flagged by zizmor as 🔒️ Proposed fix-permissions:
- contents: write
-
jobs:
update-badge:
runs-on: ubuntu-latest
+ permissions:
+ contents: write
steps:🧰 Tools🪛 zizmor (1.26.1)[error] 7-7: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level (excessive-permissions) [warning] 7-7: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||
| update-badge: | ||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||
| steps: | ||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win Pin actions to a commit SHA. Same concern as in Also applies to: 19-19, 36-36, 47-47 🧰 Tools🪛 zizmor (1.26.1)[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| persist-credentials: false | ||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Setup Python | ||||||||||||||||||||||
| uses: actions/setup-python@v5 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| python-version: "3.11" | ||||||||||||||||||||||
| cache: "pip" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Install Dependencies | ||||||||||||||||||||||
| run: | | ||||||||||||||||||||||
| python -m pip install --upgrade pip | ||||||||||||||||||||||
| pip install -r requirements.txt | ||||||||||||||||||||||
| pip install -r requirements-test.txt | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Run tests | ||||||||||||||||||||||
| run: | | ||||||||||||||||||||||
| pytest tests/ -v --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=minichain | tee pytest-coverage.txt | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Coverage comment | ||||||||||||||||||||||
| id: coverage | ||||||||||||||||||||||
| uses: MishaKav/pytest-coverage-comment@v1 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| pytest-coverage-path: ./pytest-coverage.txt | ||||||||||||||||||||||
| junitxml-path: ./pytest.xml | ||||||||||||||||||||||
| hide-comment: true | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Update README | ||||||||||||||||||||||
| run: | | ||||||||||||||||||||||
| sed -i '/<!-- Pytest Coverage Comment:Begin -->/,/<!-- Pytest Coverage Comment:End -->/c\<!-- Pytest Coverage Comment:Begin -->\n\${{ steps.coverage.outputs.coverageHtml }}\n<!-- Pytest Coverage Comment:End -->' ./README.md | ||||||||||||||||||||||
|
Comment on lines
+42
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== update-badge workflow excerpt ==\n'
nl -ba .github/workflows/update-badge.yml | sed -n '1,140p'
printf '\n== search for coverageHtml definition/usages ==\n'
rg -n "coverageHtml|coverageHtml" .github/workflows .github -SRepository: StabilityNexus/MiniChain Length of output: 239 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== update-badge workflow excerpt ==\n'
cat -n .github/workflows/update-badge.yml | sed -n '1,160p'
printf '\n== search for coverageHtml definition/usages ==\n'
rg -n "coverageHtml" .github/workflows .github -SRepository: StabilityNexus/MiniChain Length of output: 2602 🌐 Web query:
💡 Result: The pytest-coverage-comment GitHub Action by MishaKav provides a coverageHtml output, which is an HTML string containing the full coverage report with clickable links to uncovered lines [1][2]. Regarding the handling of this output as raw HTML: 1. HTML Sanitization: The coverageHtml output contains raw HTML elements (such as links and table structures). If you intend to use this output in contexts that require sanitized or safe HTML—such as displaying it in a GitHub Actions job summary—you generally do not need to sanitize it yourself if you are using standard tools like actions/github-script, which can handle raw HTML strings correctly [3]. 2. Displaying Raw HTML: If you want to include this HTML in a GitHub workflow summary, you can use the actions/github-script tool to write the raw output directly to the job summary [3]. The recommended approach is: - name: Add Summary uses: actions/github-script@v6 with: script: | core.summary.addRaw('${{ steps.coverageComment.outputs.coverageHtml }}').write 3. Troubleshooting Empty Output: If you encounter an empty coverageHtml output, ensure you are using a recent version of the action (v1.1.57 or later), as a known issue regarding empty HTML output when using XML coverage files was resolved in that version [4][5]. Additionally, ensure your pytest command generates the appropriate report files (e.g., --cov-report term-missing) to provide the necessary data for the action to parse [3]. The action also provides options like remove-links-to-files and remove-links-to-lines if you prefer a simpler text-based representation over the default HTML links [2]. Citations:
🌐 Web query:
💡 Result: In GitHub Actions, there is no direct mechanism to "escape" the Citations:
Move
🧰 Tools🪛 zizmor (1.26.1)[info] 44-44: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Commit changes | ||||||||||||||||||||||
| uses: stefanzweifel/git-auto-commit-action@v5 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| commit_message: 'docs: update coverage badge' | ||||||||||||||||||||||
| file_pattern: README.md | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| pytest>=7.0.0 | ||
| pytest-asyncio>=0.21.0 | ||
| pytest-cov>=4.0.0 |
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.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Add a concurrency group.
Without a
concurrencyblock, multiple pushes to the same PR queue redundant, overlapping test runs, wasting CI minutes and potentially producing out-of-order coverage comments.♻️ Proposed addition
on: pull_request: branches: - main + +concurrency: + group: pr-checks-${{ github.ref }} + cancel-in-progress: true📝 Committable suggestion
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 Prompt for AI Agents
Source: Linters/SAST tools