Skip to content

feat: add GitHub Actions CI workflow and contribution improvements #4237

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

Closed
dashitongzhi wants to merge 1 commit into from
Closed

feat: add GitHub Actions CI workflow and contribution improvements #4237

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 33 additions & 0 deletions .github/workflows/python-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Python CI

on:
push:
branches: [ main, master ]
pull_request:

jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.9', '3.10', '3.11', '3.12']
steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
cache: 'pip'
- name: Install dependencies
run: |
pip install -e . 2>/dev/null || pip install -r requirements.txt 2>/dev/null || true
pip install pytest pytest-cov flake8 mypy
Comment on lines +23 to +24
Comment on lines +21 to +24
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not silently ignore dependency installation failures

Line 23 masks install errors (2>/dev/null ... || true), so CI can continue with an invalid environment and produce misleading green builds.

Suggested fix 
       - name: Install dependencies
         run: |
-          pip install -e . 2>/dev/null || pip install -r requirements.txt 2>/dev/null || true
+          set -euo pipefail
+          if [ -f pyproject.toml ] || [ -f setup.py ]; then
+            pip install -e .
+          elif [ -f requirements.txt ]; then
+            pip install -r requirements.txt
+          else
+            echo "No project dependency manifest found (pyproject.toml/setup.py/requirements.txt)" >&2
+            exit 1
+          fi
           pip install pytest pytest-cov flake8 mypy
📝 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.

Suggested change
- name: Install dependencies
run: |
pip install -e . 2>/dev/null || pip install -r requirements.txt 2>/dev/null || true
pip install pytest pytest-cov flake8 mypy
- name: Install dependencies
run: |
set -euo pipefail
if [ -f pyproject.toml ] || [ -f setup.py ]; then
pip install -e .
elif [ -f requirements.txt ]; then
pip install -r requirements.txt
else
echo "No project dependency manifest found (pyproject.toml/setup.py/requirements.txt)" >&2
exit 1
fi
pip install pytest pytest-cov flake8 mypy
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/python-ci.yml around lines 21 - 24, The "Install
dependencies" step currently masks failures by redirecting stderr and using "||
true" in the run block; update the run command in the Install dependencies step
so that pip errors are not silenced (remove "2>/dev/null" and remove "|| true"),
allow the job to fail on install failures, and optionally provide a clear
fallback by testing each install command separately and exiting non-zero on
failure; locate the run block for the step named "Install dependencies" and
modify it accordingly.

- name: Lint with flake8
run: |
flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
- name: Type check with mypy
run: |
mypy . --ignore-missing-imports 2>/dev/null || echo "mypy not configured"
Comment on lines +28 to +30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Type-check step currently passes even when mypy fails

Line 30 converts any mypy non-zero exit into success via || echo ..., which disables type-check enforcement.

Suggested fix 
       - name: Type check with mypy
         run: |
-          mypy . --ignore-missing-imports 2>/dev/null || echo "mypy not configured"
+          set -euo pipefail
+          mypy . --ignore-missing-imports
📝 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.

Suggested change
- name: Type check with mypy
run: |
mypy . --ignore-missing-imports 2>/dev/null || echo "mypy not configured"
- name: Type check with mypy
run: |
set -euo pipefail
mypy . --ignore-missing-imports
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/python-ci.yml around lines 28 - 30, The Type check with
mypy step is swallowing failures by appending "|| echo 'mypy not configured'" to
the run command; remove that fallback so mypy's non-zero exit causes the job to
fail (or alternatively set the step's continue-on-error to true/false
appropriately). Locate the step named "Type check with mypy" and change the run
invocation (currently "mypy . --ignore-missing-imports 2>/dev/null || echo 'mypy
not configured'") to run mypy directly and surface its exit code (e.g., run
"mypy . --ignore-missing-imports" without the "|| echo" or handle errors via
GitHub Actions step settings).

- name: Run tests
run: |
pytest --tb=short --cov=. --cov-report=term-missing 2>/dev/null || python -m unittest discover 2>/dev/null || echo "No test framework found"
Comment on lines +31 to +33
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Test job can pass even when no tests run

Line 33 falls through to echo "No test framework found" instead of failing, so PRs can pass without executing tests.

Suggested fix 
       - name: Run tests
         run: |
-          pytest --tb=short --cov=. --cov-report=term-missing 2>/dev/null || python -m unittest discover 2>/dev/null || echo "No test framework found"
+          set -euo pipefail
+          if command -v pytest >/dev/null 2>&1; then
+            pytest --tb=short --cov=. --cov-report=term-missing
+          else
+            python -m unittest discover
+          fi
📝 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.

Suggested change
- name: Run tests
run: |
pytest --tb=short --cov=. --cov-report=term-missing 2>/dev/null || python -m unittest discover 2>/dev/null || echo "No test framework found"
- name: Run tests
run: |
set -euo pipefail
if command -v pytest >/dev/null 2>&1; then
pytest --tb=short --cov=. --cov-report=term-missing
else
python -m unittest discover
fi
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/python-ci.yml around lines 31 - 33, The "Run tests" step
currently falls back to echoing "No test framework found" which allows the job
to pass even when no tests run; update the step's run command so that it runs
pytest, then falls back to python -m unittest only if pytest is not available,
and if both commands are unavailable or both return non-zero, ensure the shell
returns a non-zero exit (e.g., replace the final echo fallback with an explicit
failure/exit) so the job fails; look for the Run tests step and the existing
command string ("pytest --tb=short --cov=. --cov-report=term-missing 2>/dev/null
|| python -m unittest discover 2>/dev/null || echo \"No test framework found\"")
and change its control flow to propagate a non-zero exit when neither test
runner succeeds.

Loading