-
Notifications
You must be signed in to change notification settings - Fork 0
Eng#2: Lock CI gates and publish QA #5
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,101 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: perf-light | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pull_request: {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| k6: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 15 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| concurrency: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| group: perf-${{ github.ref }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cancel-in-progress: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Install k6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sudo apt-get update | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sudo apt-get install -y k6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Start stack | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd paform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| docker compose --env-file .env.development -f docker-compose.dev.yml up -d --build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Each step in the new perf-light workflow starts with Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Wait for API | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd paform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in {1..60}; do curl -sf http://localhost:8000/healthcheck && break || sleep 2; done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Wait for Frontend | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in {1..60}; do curl -sf http://localhost:3000/models/manifest.json && break || sleep 2; done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider adding failure handling for healthchecks. The healthcheck loops retry 60 times with 2-second intervals (2 minutes total), but silently succeed if the service never responds. Consider failing explicitly if the loop exhausts retries. Apply this diff to fail explicitly: - name: Wait for API
run: |
cd paform
- for i in {1..60}; do curl -sf http://localhost:8000/healthcheck && break || sleep 2; done
+ for i in {1..60}; do
+ curl -sf http://localhost:8000/healthcheck && break || sleep 2
+ if [ $i -eq 60 ]; then echo "API healthcheck failed after 2m"; exit 1; fi
+ done
- name: Wait for Frontend
run: |
- for i in {1..60}; do curl -sf http://localhost:3000/models/manifest.json && break || sleep 2; done
+ for i in {1..60}; do
+ curl -sf http://localhost:3000/models/manifest.json && break || sleep 2
+ if [ $i -eq 60 ]; then echo "Frontend manifest check failed after 2m"; exit 1; fi
+ done📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Seed backend for perf | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BASE_URL: http://localhost:8000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd paform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python - <<'PY' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import urllib.error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import urllib.request | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BASE_URL = os.environ.get("BASE_URL", "http://localhost:8000") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def post(path: str, payload: dict) -> dict: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req = urllib.request.Request( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"{BASE_URL}{path}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data=json.dumps(payload).encode("utf-8"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Content-Type": "application/json"}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with urllib.request.urlopen(req, timeout=10) as resp: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return json.loads(resp.read().decode("utf-8")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except urllib.error.HTTPError as exc: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail = exc.read().decode("utf-8", "ignore") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise SystemExit(f"Seed request failed ({exc.code}): {detail}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| material = post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/api/materials/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {"name": "Walnut", "texture_url": None, "cost_per_sq_ft": 12.5}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| material_id = material.get("id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not material_id: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise SystemExit("Material creation failed; missing id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/api/modules/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "name": "Base600", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "width": 600.0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "height": 720.0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "depth": 580.0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "base_price": 100.0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "material_id": material_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PY | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Run k6 light profile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BASE_URL: http://localhost:8000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FRONTEND_BASE_URL: http://localhost:3000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd paform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| k6 run --summary-export k6-summary.json tests/perf/k6-quote-cnc.js | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Upload k6 summary | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: always() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: k6-summary | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: paform/k6-summary.json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Shutdown stack | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: always() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cd paform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| docker compose --env-file .env.development -f docker-compose.dev.yml down | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from fastapi.testclient import TestClient | ||
|
|
||
| from api.main import app | ||
|
|
||
|
|
||
| client = TestClient(app) | ||
|
|
||
|
|
||
| def _is_error_shape(payload: dict) -> bool: | ||
| if not isinstance(payload, dict): | ||
| return False | ||
| if payload.get('ok') is not False: | ||
| return False | ||
| error = payload.get('error') | ||
| return isinstance(error, dict) and 'code' in error and 'message' in error | ||
|
|
||
|
|
||
| def test_quote_invalid_payload_envelope() -> None: | ||
| response = client.post( | ||
| '/api/quote/generate', | ||
| data='not-json', | ||
| headers={'Content-Type': 'application/json'}, | ||
| ) | ||
| assert response.status_code in (400, 422) | ||
| assert _is_error_shape(response.json()) | ||
|
|
||
|
|
||
| def test_cnc_invalid_payload_envelope() -> None: | ||
| response = client.post( | ||
| '/api/cnc/export', | ||
| data='not-json', | ||
| headers={'Content-Type': 'application/json'}, | ||
| ) | ||
| assert response.status_code in (400, 422) | ||
| assert _is_error_shape(response.json()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Optional style fix for consistency.
The
pull_request: {}syntax is valid butpull_request:(no empty dict) is more conventional for workflows that trigger on all PR events.Apply this diff for consistency:
📝 Committable suggestion
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
🤖 Prompt for AI Agents