Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds a new local CI suite (multiple scripts for orchestration, E2E, mutation testing, smoke, Sonar), narrows gitignore entries for scripts, updates docs references, introduces a runtime-checkable FuzzyNumber Protocol with a test, and tweaks Docker build caching and frontend healthcheck. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Local Runner
participant Docker as Postgres Container
participant API as API Server
participant Tests as Test Runners
Runner->>Docker: start postgres container
Docker-->>Runner: container ready
Runner->>API: start API with DB env
API-->>Runner: health endpoint ready
Runner->>Tests: invoke selected suites (backend / playwright / visual)
Tests->>API: exercise endpoints / UI
Tests->>Docker: perform DB queries
Tests-->>Runner: return exit codes
Runner->>API: stop API process
Runner->>Docker: remove postgres container
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
caitlon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ci/e2e-local.sh`:
- Around line 8-12: Update the header comment in scripts/ci/e2e-local.sh to
accurately describe the "all" mode: change the line that currently reads "#
./scripts/ci/e2e-local.sh # all E2E tests (backend + Playwright)"
to mention that "all" runs backend, Playwright, and visual regression (Chromium)
so it matches the actual behavior of the "all" option used later in the script.
In `@scripts/ci/mutmut-run.sh`:
- Line 40: Replace the bc-based percentage calculation in the echo line with an
awk-based calculation to avoid dependency on bc; use the existing killed and
total variables (killed and total) inside the echo command and call awk to
compute killed * 100 / total with one decimal of precision (and guard against
total == 0 by outputting 0.0% or similar). Update the echo that currently uses
"$(echo "scale=1; $killed * 100 / $total" | bc)" so it instead invokes awk with
the shell-expanded killed and total to produce a one-decimal percentage.
In `@scripts/ci/smoke-test.sh`:
- Around line 28-49: Replace the terse "A && B || C" chains used for each smoke
test check with explicit if-then-else conditionals to make the logic correct and
readable: for the API health check (variables HEALTH, CHECKS, pass, fail,
API_URL), wrap the curl/grep sequence in if ... then pass "API health endpoint"
else fail "API health endpoint ($API_URL/health)" fi; do the same for the
calculate check (CALC_BODY, CALC, pass, fail, API_URL) and the frontend load
check (curl, pass, fail, FRONTEND_URL), ensuring CHECKS is incremented
beforehand and that failures are only triggered by the actual curl/grep test
result.
- Around line 1-2: Add Bash strict mode to the top of the script by enabling
"set -euo pipefail" immediately after the existing shebang (#!/usr/bin/env bash)
so the script fails fast on errors, treats unset variables as errors, and
propagates failures through pipes; update any functions or commands in the
script (if present) to be compatible with this stricter behavior (e.g., ensure
variables are initialized before use).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
.gitignoredocs/quality-report.mdscripts/ci/ci-local.shscripts/ci/e2e-local.shscripts/ci/mutmut-run.shscripts/ci/smoke-test.shscripts/ci/sonar-local.sh
| total=$((killed + survived)) | ||
|
|
||
| if [ "$total" -gt 0 ]; then | ||
| echo "Mutation score: $killed / $total = $(echo "scale=1; $killed * 100 / $total" | bc)%" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider awk instead of bc for broader compatibility.
bc may not be available in minimal container environments. awk is more universally available.
♻️ Proposed fix
if [ "$total" -gt 0 ]; then
- echo "Mutation score: $killed / $total = $(echo "scale=1; $killed * 100 / $total" | bc)%"
+ echo "Mutation score: $killed / $total = $(awk "BEGIN {printf \"%.1f\", $killed * 100 / $total}")%"
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
| echo "Mutation score: $killed / $total = $(echo "scale=1; $killed * 100 / $total" | bc)%" | |
| echo "Mutation score: $killed / $total = $(awk "BEGIN {printf \"%.1f\", $killed * 100 / $total}")%" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ci/mutmut-run.sh` at line 40, Replace the bc-based percentage
calculation in the echo line with an awk-based calculation to avoid dependency
on bc; use the existing killed and total variables (killed and total) inside the
echo command and call awk to compute killed * 100 / total with one decimal of
precision (and guard against total == 0 by outputting 0.0% or similar). Update
the echo that currently uses "$(echo "scale=1; $killed * 100 / $total" | bc)" so
it instead invokes awk with the shell-expanded killed and total to produce a
one-decimal percentage.
Comment on lines
+1
to
+2
| #!/usr/bin/env bash | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add strict mode for consistency with other CI scripts.
Other scripts in this PR (ci-local.sh, e2e-local.sh, sonar-local.sh, mutmut-run.sh) use set -euo pipefail. This script should follow the same pattern for consistency and to catch unset variable errors early.
♻️ Proposed fix
#!/usr/bin/env bash
+set -euo pipefail
# Lightweight smoke test for post-deployment verification.📝 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
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash | |
| set -euo pipefail | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ci/smoke-test.sh` around lines 1 - 2, Add Bash strict mode to the top
of the script by enabling "set -euo pipefail" immediately after the existing
shebang (#!/usr/bin/env bash) so the script fails fast on errors, treats unset
variables as errors, and propagates failures through pipes; update any functions
or commands in the script (if present) to be compatible with this stricter
behavior (e.g., ensure variables are initialized before use).
Comment on lines
+28
to
+49
| # --- Check 1: API Health --- | ||
| CHECKS=$((CHECKS + 1)) | ||
| HEALTH=$(curl -sf --max-time 5 "$API_URL/health" 2>/dev/null) && \ | ||
| echo "$HEALTH" | grep -q '"ok"' && \ | ||
| pass "API health endpoint" || \ | ||
| fail "API health endpoint ($API_URL/health)" | ||
|
|
||
| # --- Check 2: Calculate endpoint --- | ||
| CHECKS=$((CHECKS + 1)) | ||
| CALC_BODY='{"experts":[{"name":"A","lower":5.0,"peak":10.0,"upper":15.0},{"name":"B","lower":8.0,"peak":12.0,"upper":18.0}]}' | ||
| CALC=$(curl -sf --max-time 5 -X POST "$API_URL/calculate" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "$CALC_BODY" 2>/dev/null) && \ | ||
| echo "$CALC" | grep -q 'best_compromise' && \ | ||
| pass "Calculate endpoint" || \ | ||
| fail "Calculate endpoint ($API_URL/calculate)" | ||
|
|
||
| # --- Check 3: Frontend loads --- | ||
| CHECKS=$((CHECKS + 1)) | ||
| curl -sf --max-time 10 -o /dev/null "$FRONTEND_URL" 2>/dev/null && \ | ||
| pass "Frontend loads" || \ | ||
| fail "Frontend ($FRONTEND_URL)" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using explicit if-then-else for clarity.
The A && B || C pattern isn't a true if-then-else—if B fails, C runs even when A succeeds. Here pass() only echoes so it won't fail in practice, but explicit conditionals are more robust and readable.
♻️ Example refactor for Check 1
# --- Check 1: API Health ---
CHECKS=$((CHECKS + 1))
-HEALTH=$(curl -sf --max-time 5 "$API_URL/health" 2>/dev/null) && \
- echo "$HEALTH" | grep -q '"ok"' && \
- pass "API health endpoint" || \
- fail "API health endpoint ($API_URL/health)"
+if HEALTH=$(curl -sf --max-time 5 "$API_URL/health" 2>/dev/null) && \
+ echo "$HEALTH" | grep -q '"ok"'; then
+ pass "API health endpoint"
+else
+ fail "API health endpoint ($API_URL/health)"
+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
| # --- Check 1: API Health --- | |
| CHECKS=$((CHECKS + 1)) | |
| HEALTH=$(curl -sf --max-time 5 "$API_URL/health" 2>/dev/null) && \ | |
| echo "$HEALTH" | grep -q '"ok"' && \ | |
| pass "API health endpoint" || \ | |
| fail "API health endpoint ($API_URL/health)" | |
| # --- Check 2: Calculate endpoint --- | |
| CHECKS=$((CHECKS + 1)) | |
| CALC_BODY='{"experts":[{"name":"A","lower":5.0,"peak":10.0,"upper":15.0},{"name":"B","lower":8.0,"peak":12.0,"upper":18.0}]}' | |
| CALC=$(curl -sf --max-time 5 -X POST "$API_URL/calculate" \ | |
| -H "Content-Type: application/json" \ | |
| -d "$CALC_BODY" 2>/dev/null) && \ | |
| echo "$CALC" | grep -q 'best_compromise' && \ | |
| pass "Calculate endpoint" || \ | |
| fail "Calculate endpoint ($API_URL/calculate)" | |
| # --- Check 3: Frontend loads --- | |
| CHECKS=$((CHECKS + 1)) | |
| curl -sf --max-time 10 -o /dev/null "$FRONTEND_URL" 2>/dev/null && \ | |
| pass "Frontend loads" || \ | |
| fail "Frontend ($FRONTEND_URL)" | |
| # --- Check 1: API Health --- | |
| CHECKS=$((CHECKS + 1)) | |
| if HEALTH=$(curl -sf --max-time 5 "$API_URL/health" 2>/dev/null) && \ | |
| echo "$HEALTH" | grep -q '"ok"'; then | |
| pass "API health endpoint" | |
| else | |
| fail "API health endpoint ($API_URL/health)" | |
| fi | |
| # --- Check 2: Calculate endpoint --- | |
| CHECKS=$((CHECKS + 1)) | |
| CALC_BODY='{"experts":[{"name":"A","lower":5.0,"peak":10.0,"upper":15.0},{"name":"B","lower":8.0,"peak":12.0,"upper":18.0}]}' | |
| CALC=$(curl -sf --max-time 5 -X POST "$API_URL/calculate" \ | |
| -H "Content-Type: application/json" \ | |
| -d "$CALC_BODY" 2>/dev/null) && \ | |
| echo "$CALC" | grep -q 'best_compromise' && \ | |
| pass "Calculate endpoint" || \ | |
| fail "Calculate endpoint ($API_URL/calculate)" | |
| # --- Check 3: Frontend loads --- | |
| CHECKS=$((CHECKS + 1)) | |
| curl -sf --max-time 10 -o /dev/null "$FRONTEND_URL" 2>/dev/null && \ | |
| pass "Frontend loads" || \ | |
| fail "Frontend ($FRONTEND_URL)" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 31-31: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 41-41: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 47-47: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ci/smoke-test.sh` around lines 28 - 49, Replace the terse "A && B ||
C" chains used for each smoke test check with explicit if-then-else conditionals
to make the logic correct and readable: for the API health check (variables
HEALTH, CHECKS, pass, fail, API_URL), wrap the curl/grep sequence in if ... then
pass "API health endpoint" else fail "API health endpoint ($API_URL/health)" fi;
do the same for the calculate check (CALC_BODY, CALC, pass, fail, API_URL) and
the frontend load check (curl, pass, fail, FRONTEND_URL), ensuring CHECKS is
incremented beforehand and that failures are only triggered by the actual
curl/grep test result.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/docker-compose.yml`:
- Around line 94-99: The docker-compose healthcheck uses curl against the nginx
container but nginx:alpine does not include curl; update the healthcheck so it
won't fail at runtime by either (A) adding curl to the production image (modify
the Dockerfile referenced by the nginx image to install curl in the final stage)
or (B) changing the docker-compose healthcheck to a tool-free check (e.g., use
"CMD-SHELL" with "wget -q -O- http://localhost/ || exit 1" only if wget is
present, or better: use "CMD" with ["CMD", "nginx", "-t"] or a simple TCP check
like "CMD-SHELL", "cat < /dev/null > /dev/tcp/localhost/80 || exit 1") — choose
one approach and update the healthcheck test and/or Dockerfile accordingly to
ensure the command exists in the nginx:alpine runtime.
In `@scripts/ci/e2e-local.sh`:
- Line 21: The script currently assigns MODE="${1:-all}" but never validates it,
so unknown values silently do nothing; add validation after MODE is set (in
scripts/ci/e2e-local.sh) by checking MODE against the allowed set used in the
suites block (the known modes handled in lines 104–125) and, if MODE is not one
of those values or "all", print an error and exit with a non-zero status;
implement this as a simple case/esac or membership test referencing the MODE
variable so invalid inputs like "playwrit" fail fast.
- Around line 31-33: The script's checks only look for running containers
(docker ps) so a stopped container named by CONTAINER_NAME can block docker run;
update both the cleanup block that removes existing containers and the pre-run
check before docker run to search for containers regardless of state using
docker ps -a -q -f name="$CONTAINER_NAME" and remove any matches (docker rm -f
"$CONTAINER_NAME") before proceeding; adjust the commands around the
CONTAINER_NAME variable usage and any echo/log messages accordingly to ensure
stopped containers are handled.
- Around line 151-158: The script currently sums exit codes into TOTAL_EXIT
(using BACKEND_EXIT, PLAYWRIGHT_EXIT, VISUAL_EXIT), which can produce misleading
non-zero values; change the logic so the final exit is 0 only when all suites
succeeded and non-zero when any suite failed — e.g., compute a boolean result by
checking if any of BACKEND_EXIT, PLAYWRIGHT_EXIT, or VISUAL_EXIT is non-zero and
set the final exit to 1 on any failure (otherwise 0); update the TOTAL_EXIT
handling and the final exit $TOTAL_EXIT usage accordingly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
docker/Dockerfiledocker/docker-compose.ymlscripts/ci/e2e-local.shsrc/models/fuzzy_number.pytests/unit/models/test_fuzzy_number.py
Comment on lines
+94
to
+99
| healthcheck: | ||
| test: ["CMD-SHELL", "curl -f http://localhost:80/ || exit 1"] | ||
| interval: 30s | ||
| timeout: 10s | ||
| retries: 3 | ||
| start_period: 10s |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Dockerfile" -path "*/frontend/*" -type fRepository: caitlon/BeCoMe
Length of output: 79
🏁 Script executed:
cat -n ./frontend/DockerfileRepository: caitlon/BeCoMe
Length of output: 1190
Add curl to the nginx:alpine image or use an alternative healthcheck method that doesn't require external tools.
The frontend Dockerfile uses nginx:alpine as the production base image, which does not include curl by default. The healthcheck will fail at runtime because the curl command won't be found, causing the container to fail health checks.
Either install curl in the production stage:
FROM nginx:alpine
+RUN apk add --no-cache curl
COPY --from=builder /app/dist /usr/share/nginx/htmlOr use an alternative that requires no external tools:
healthcheck:
- test: ["CMD-SHELL", "curl -f http://localhost:80/ || exit 1"]
+ test: ["CMD-SHELL", "(echo -e 'GET / HTTP/1.0\\r\\n\\r\\n' > /dev/tcp/localhost/80) 2>/dev/null || exit 1"]
interval: 30s📝 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
| healthcheck: | |
| test: ["CMD-SHELL", "curl -f http://localhost:80/ || exit 1"] | |
| interval: 30s | |
| timeout: 10s | |
| retries: 3 | |
| start_period: 10s | |
| healthcheck: | |
| test: ["CMD-SHELL", "(echo -e 'GET / HTTP/1.0\\r\\n\\r\\n' > /dev/tcp/localhost/80) 2>/dev/null || exit 1"] | |
| interval: 30s | |
| timeout: 10s | |
| retries: 3 | |
| start_period: 10s |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/docker-compose.yml` around lines 94 - 99, The docker-compose
healthcheck uses curl against the nginx container but nginx:alpine does not
include curl; update the healthcheck so it won't fail at runtime by either (A)
adding curl to the production image (modify the Dockerfile referenced by the
nginx image to install curl in the final stage) or (B) changing the
docker-compose healthcheck to a tool-free check (e.g., use "CMD-SHELL" with
"wget -q -O- http://localhost/ || exit 1" only if wget is present, or better:
use "CMD" with ["CMD", "nginx", "-t"] or a simple TCP check like "CMD-SHELL",
"cat < /dev/null > /dev/tcp/localhost/80 || exit 1") — choose one approach and
update the healthcheck test and/or Dockerfile accordingly to ensure the command
exists in the nginx:alpine runtime.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a suite of new developer scripts to streamline local CI, E2E testing, mutation testing, SonarCloud analysis, and smoke testing. These scripts provide a unified interface for running and debugging the project's quality and verification pipelines outside of GitHub Actions. Additionally, the documentation is updated to reflect the new script locations.
New developer tooling and CI scripts:
ci-local.sh: a unified local runner for linting, testing, E2E, smoke, mutation, and SonarCloud analysis, mirroring the GitHub Actions workflow.e2e-local.sh: a full-stack E2E test runner that manages PostgreSQL via Docker, handles API startup, and runs both backend and Playwright frontend E2E tests, with cleanup and reporting.mutmut-run.sh: a script for running mutation testing on thesrc/module, including commands for summary and detailed surviving mutant reports.sonar-local.sh: a script for running SonarCloud analysis locally on the whole project or specific modules, with optional coverage generation and token management.smoke-test.sh: a lightweight script for post-deployment verification, checking API health, calculation endpoint, and frontend availability.Documentation updates:
docs/quality-report.mdto reference the new script location underscripts/ci/.