feat: expand CI/CD pipeline with additional test coverage#168
feat: expand CI/CD pipeline with additional test coverage#168bugman-007 wants to merge 1 commit intoLight-Heart-Labs:mainfrom
Conversation
2296f98 to
06c3cab
Compare
06c3cab to
6124805
Compare
Lightheartdevs
left a comment
There was a problem hiding this comment.
Request Changes
The PR adds useful CI infrastructure but has issues that could cause problems.
What's good
- Adding service registry tests to the Linux CI workflow
- Path-scoped triggers (only runs when relevant files change)
Issues
1. mypy with || true — every step passes unconditionally
All four mypy steps end with || true:
mypy . --ignore-missing-imports --no-strict-optional --check-untyped-defs || trueThis means the type-check workflow can never fail. It will always show green regardless of how many type errors exist. Either:
- Remove
|| trueand fix existing type errors, or - If the goal is informational-only, add
continue-on-error: trueat the step level so GitHub shows it as a warning (orange) rather than silently passing
2. Pre-commit config adds Black + Ruff formatting enforcement
Adding black (formatting) and ruff (linting) as pre-commit hooks will reject commits from every existing contributor who doesn't have pre-commit installed. The existing codebase has never been Black-formatted, so running black --line-length=100 across the Python files will produce a massive formatting diff.
This needs a plan:
- Run
blackonce on the entire codebase in a dedicated formatting PR - Then add the pre-commit hook
Otherwise every contributor's next PR will fail pre-commit and they'll need to either format everything or skip hooks.
3. shellcheck with --severity=error — This is fine as a starting point, but SC1091 (can't follow sourced file) is already excluded. Worth also excluding SC2086 (word splitting) since the codebase uses unquoted variables intentionally in many places for readability.
Recommendation
Split into:
- PR A: Add service registry tests to CI (the
.github/workflows/test-linux.ymlchange) — merge now - PR B: The mypy workflow — fix the
|| trueissue first - PR C: Pre-commit config — needs a formatting baseline PR first
Adds test-service-registry.sh to the CI pipeline to validate service registry functionality on every PR and push to main. Related: Light-Heart-Labs#168
|
Closing this PR as it has been split into 4 separate PRs per @Lightheartdevs feedback:
This addresses all the issues raised in the review. |
|
Closing this PR as it has been split into 4 separate PRs per @Lightheartdevs feedback:
This addresses all the issues raised in the review. |
Add comprehensive test execution and code quality checks:
Benefits
Testing
All workflows are configured to run on:
The workflows use existing test scripts and tools, so no new dependencies are introduced.