fix: CI workflow - add setup steps, split test jobs, add coverage upload#2
Conversation
- Add actions/setup-python@v5 for Python 3.12 - Add actions/setup-node@v4 for Node 20 - Install backend and analytics deps before lint/test - Split test job into test-backend and test-analytics - Use npm ci for frontend dependency install - Add ruff for Python linting - Add Buildx for Docker image builds with caching - Add coverage artifact uploads - Add release notes generation - Remove npm test (no test script in frontend package.json)
sam1064max
left a comment
There was a problem hiding this comment.
Code Review: CI Workflow Fix
lint job L90: 🔴 bug: tests import from \�pp.xxx\ which is under \�ackend/app/. Running \pytest backend/tests/\ from repo root won't resolve \�pp\ module. Add \PYTHONPATH=backend\ or use \working-directory: backend.
** est-analytics job L110:** 🔴 bug: same module resolution issue. Analytics tests import from \�pp.path_analyzer\ which is under \�nalytics/app/. Use \working-directory: analytics\ or set \PYTHONPATH=analytics.
lint job L87: 🟡 risk:
pm ci\ requires \package-lock.json\ which may not be committed. If lock file missing,
pm ci\ fails. Use
pm install\ as fallback or verify lock file exists.
lint job L93: 🔵 nit:
uff check\ only lints, doesn't enforce formatting. Add
uff format --check backend/ analytics/\ before lint step.
** est-backend job L105:** 🔵 nit: \coverage.xml\ path ambiguous. pytest generates it in CWD. Since no \working-directory, it lands at repo root. Upload artifact path should match. Confirmed OK as-is since both use root.
�uild job L128: 🔵 nit: build jobs depend on lint+test but no caching across jobs. \setup-python\ cache helps pip, but full dependency install runs on every test job. Acceptable for POC, consider composite actions in future.
Summary
Two critical bugs (import path resolution), one risk (npm ci), two nits. Fix import paths and the rest LGTM.
Description
Fixes CI workflow failing due to missing setup steps and incorrect job configuration.
Changes
actions/setup-python@v5(Python 3.12) to all Python jobsactions/setup-node@v4(Node 20) to lint jobtestjob intotest-backendandtest-analyticsfor parallelismrequirements.txtbefore lint/testnpm ciinstead of ad-hocnpm install eslintdocker buildwithdocker/build-push-action@v5(Buildx caching)actions/upload-artifact@v4softprops/action-gh-release@v2with auto release notesnpm test(no test script in frontend package.json; typecheck covers validation)Closes #1