feat: setup github actions #757
Conversation
WalkthroughThis update introduces a new GitHub Actions workflow for building and testing the project, adds a Docker Compose configuration for core services (Redis, Memcached, Mailpit), and refines a CLI smoke test script by adjusting Docker command flags and regular expressions for version checks. No public code entities are altered. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Docker Compose
participant PDM
participant Make
participant CLI Smoke Test
GitHub Actions->>GitHub Actions: Trigger on pull_request
GitHub Actions->>Make: Run 'make comply'
GitHub Actions->>Docker Compose: Start services (redis, memcached, mailpit)
GitHub Actions->>Make: Run 'make test'
GitHub Actions->>Make: Run 'make test' (matrix: OS/Python)
GitHub Actions->>Docker Compose: Start services for CLI smoke test
GitHub Actions->>CLI Smoke Test: Execute cli-smoke-test.sh
CLI Smoke Test->>Docker Compose: Run commands in containers
CLI Smoke Test->>GitHub Actions: Output test results
Poem
✨ Finishing Touches
🪧 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.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docker/compose-services-only.yml (1)
4-34: Add healthchecks for service readiness
Currently there are nohealthcheckdefinitions. Adding healthchecks forredis,memcached, andmailpitensures that dependent workflows can wait for services to be fully available before running tests. For example:services: redis: image: redis:7.2.6 + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 10s + timeout: 5s + retries: 5 memcached: image: memcached:1.6.20 + healthcheck: + test: ["CMD", "echo", "stats", "|", "nc", "-w", "2", "localhost", "11211"] + interval: 10s + timeout: 5s + retries: 5 mailpit: image: axllent/mailpit:1.0.0 + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:8025/"] + interval: 10s + timeout: 5s + retries: 5scripts/cli-smoke-test.sh (2)
2-2: Ensure pipeline failures are caught
Addset -o pipefailafterset -eso that failures inside pipelines (e.g., agrepthat doesn’t match) correctly cause the script to exit with a non-zero status.
18-18: Use modern command substitution
Replace backticks with$(...)for consistency and readability:- -v `pwd`:/src \ + -v $(pwd):/src \🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 18-18: Quote this to prevent word splitting.
(SC2046)
.github/workflows/build_and_test.yml (1)
65-67: Clean up or implement commented matrix entries
The# FIXME ?and commented-out OS matrix entries can be confusing. Either remove them or enable multi-OS testing when ready.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build_and_test.yml(1 hunks)docker/compose-services-only.yml(1 hunks)scripts/cli-smoke-test.sh(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: test-all (ubuntu-latest, pypy3.10)
- GitHub Check: test-all (ubuntu-latest, 3.13)
- GitHub Check: test-all (ubuntu-latest, 3.12)
- GitHub Check: test-all (ubuntu-latest, 3.11)
- GitHub Check: test-all (ubuntu-latest, 3.9)
- GitHub Check: test-all (ubuntu-latest, 3.8)
- GitHub Check: Travis CI - Pull Request
- GitHub Check: Travis CI - Branch
🔇 Additional comments (2)
.github/workflows/build_and_test.yml (2)
3-6: Approved: Minimal explicit permissions
The globalpermissionsblock now restricts theGITHUB_TOKENtocontents: readandpull-requests: write, addressing the security scanner’s recommendations.
91-94: Verify compose file consistency for smoke tests
Thecli-smoke-testjob usesdocker-compose.yml, whereas other jobs usedocker/compose-services-only.yml. Ensure the CLI smoke test has access to the intended service definitions (Redis, Memcached, Mailpit), or adjust the path to the correct compose file.
| restart: always | ||
| volumes: | ||
| - mailpit-data:/data | ||
| - ../docker/mailpit:/certificates |
There was a problem hiding this comment.
Avoid relative host paths for certificates
The host path ../docker/mailpit is relative to where docker-compose is invoked and may not exist in all contexts. Consider using an absolute path or mounting via a named volume with a consistent directory structure to avoid CI failures.
| image: redis:latest | ||
| hostname: redis |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin images to fixed versions
Using latest (and an unqualified axllent/mailpit) can lead to unexpected breaking changes when upstream images are updated. Specify explicit, semantically-versioned tags (e.g., redis:7.2.6, memcached:1.6.20, axllent/mailpit:1.0.0) and update them deliberately.
Also applies to: 12-13, 18-19
|
|
||
| docker exec -it cement-cli-smoke-test /bin/bash -c "cd /src ; pip install `ls dist/cement-*.tar.gz`[cli]" | ||
| tmp=$(docker exec cement-cli-smoke-test /bin/bash -c "mktemp -d") | ||
| docker exec cement-cli-smoke-test /bin/bash -c "cd /src ; pip install `ls dist/cement-*.tar.gz`[cli]" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify package installation command
Avoid using ls and backticks for glob resolution. You can rely on shell globbing:
- docker exec cement-cli-smoke-test /bin/bash -c "cd /src ; pip install `ls dist/cement-*.tar.gz`[cli]"
+ docker exec cement-cli-smoke-test /bin/bash -c "cd /src ; pip install dist/cement-*.tar.gz[cli]"Committable suggestion skipped: line range outside the PR's diff.
Adds Github Actions to replace Travis CI.
Summary by CodeRabbit