LCORE-306: setup e2e test env #306
Conversation
|
""" WalkthroughThis change adds a new end-to-end (E2E) testing workflow using GitHub Actions that orchestrates multi-service Docker Compose containers. It introduces a Docker Compose configuration for Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Docker Compose
participant llama-stack
participant lightspeed-stack
participant Test Runner
GitHub Actions->>GitHub Actions: Checkout repository code
GitHub Actions->>GitHub Actions: Create config files (run.yaml, lightspeed-stack.yaml)
GitHub Actions->>Docker Compose: Start llama-stack and lightspeed-stack services
Docker Compose->>llama-stack: Build and run container
Docker Compose->>lightspeed-stack: Pull image and run container after llama-stack
GitHub Actions->>GitHub Actions: Wait 20 seconds for service health
GitHub Actions->>lightspeed-stack: Perform connectivity test (curl local endpoint)
GitHub Actions->>GitHub Actions: Install Python dependencies
GitHub Actions->>Test Runner: Run end-to-end test suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![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
🧹 Nitpick comments (7)
test.containerfile (2)
13-15: Curl-pipe-to-sh without verificationPiping an external script directly to
shbypasses integrity checks.At minimum fetch first and verify its checksum / GPG sig, e.g.:
curl -LsSf -o /tmp/uv-install.sh https://astral.sh/uv/install.sh \ && echo "<expected-sha256> /tmp/uv-install.sh" | sha256sum -c - \ && sh /tmp/uv-install.sh
37-37: Port not exposed – minor DX issueAdd
EXPOSE 8321so localdocker run -pusers see which port the service expects.+EXPOSE 8321docker-compose.yaml (2)
1-7: Compose version header missingWhile optional in Compose v2 CLI, many tools (including some IDEs & CI linters) still expect a
version: "3.9"(or newer) field.
Recommend adding it for forward compatibility.
30-32: No newline at EOFYAML-lint flags this. Add a final newline to keep linters quiet.
driver: bridge +.github/workflows/e2e_tests.yaml (3)
15-21: Indentation & trailing-space lint errorsLines 16–17 produce YAML-lint errors. Align
env:two spaces under- uses:and strip trailing whitespace.- env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}Apply the same fix to the second
create-a-file-actionblock and lines 196-203.
211-218: Connectivity test may pass while services are still unhealthyRelying on fixed
sleep 20can be flaky on slow runners. Prefer Docker health-checks indocker-compose.yamland poll fordocker compose ps --format "{{.State.Health}}".
220-227: Missing cleanup step
docker compose up -dleaves containers dangling after the job. Adddocker compose down -vinfinally:or apoststep to free runner disk space and avoid side effects on parallel jobs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/e2e_tests.yaml(1 hunks)docker-compose.yaml(1 hunks)test.containerfile(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
docker-compose.yaml
[error] 32-32: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/e2e_tests.yaml
[error] 16-16: trailing spaces
(trailing-spaces)
[warning] 17-17: wrong indentation: expected 10 but found 12
(indentation)
[error] 57-57: trailing spaces
(trailing-spaces)
[warning] 58-58: wrong indentation: expected 10 but found 12
(indentation)
[error] 196-196: trailing spaces
(trailing-spaces)
[warning] 197-197: wrong indentation: expected 10 but found 12
(indentation)
[error] 202-202: trailing spaces
(trailing-spaces)
[error] 227-227: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
test.containerfile (1)
17-35: Huge image & long build – slim it downInstalling 30 + Python packages inside the image balloons its size (>3 GB) and slows CI.
Typical optimisations:
- Add
--no-cache-dirtouv pip install …- Combine with
--systemso the venv isn’t needed, or copy a pre-built wheelhouse.- Remove build deps (
python*-devel,gcc, etc.) in a later layer viamicrodnf removeto keep runtimes lean.[ suggest_optional_refactor ]
docker-compose.yaml (1)
10-14: Host path differs from container default
/app-root/run.yamlis mounted but the container currently looks forrun.yamlin/.
After applying theWORKDIRchange suggested for the Dockerfile this mismatch disappears; otherwise add the correct path inCMD..github/workflows/e2e_tests.yaml (1)
195-205:uv synclikely no-op / failure
uv syncexpects apyproject.tomlto resolve dependencies; none exists in the repo.
If the intention is to install test deps, justpip install -r requirements-e2e.txt(or similar).
|
Are the e2e tests meant to be running or will they run only once this PR is merged? |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/e2e_tests.yaml (1)
4-11: Security trade-off forpull_request_targetnoted
Previous reviews already covered the risk of exposing repository secrets to untrusted PRs. Your explicit preference for unrestricted testing is acknowledged, so no further action is proposed here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e_tests.yaml(1 hunks)test.containerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test.containerfile
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: radofuchs
PR: lightspeed-core/lightspeed-stack#306
File: .github/workflows/e2e_tests.yaml:4-4
Timestamp: 2025-07-28T13:15:54.882Z
Learning: The user radofuchs prefers unrestricted pull_request_target workflows with secrets access for comprehensive PR testing, accepting the security trade-offs to ensure all problems are caught during the PR process.
.github/workflows/e2e_tests.yaml (1)
Learnt from: radofuchs
PR: #306
File: .github/workflows/e2e_tests.yaml:4-4
Timestamp: 2025-07-28T13:15:54.882Z
Learning: The user radofuchs prefers unrestricted pull_request_target workflows with secrets access for comprehensive PR testing, accepting the security trade-offs to ensure all problems are caught during the PR process.
🪛 YAMLlint (1.37.1)
.github/workflows/e2e_tests.yaml
[error] 18-18: trailing spaces
(trailing-spaces)
[warning] 19-19: wrong indentation: expected 10 but found 12
(indentation)
[error] 59-59: trailing spaces
(trailing-spaces)
[warning] 60-60: wrong indentation: expected 10 but found 12
(indentation)
[error] 198-198: trailing spaces
(trailing-spaces)
[warning] 199-199: wrong indentation: expected 10 but found 12
(indentation)
[error] 204-204: trailing spaces
(trailing-spaces)
[error] 229-229: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
.github/workflows/e2e_tests.yaml (2)
13-15: Good catch-out of PR SHA – ensures the PR code is actually tested
Specifyingref: ${{ github.event.pull_request.head.sha }}fixes the “base branch only” pitfall forpull_request_target.
225-229: Add missing newline at EOF & avoid implicitpip install uvsurprise
- GitHub spits a warning for files without a newline.
uvis a Rust binary; installing viapipsometimes pulls an outdated shim. Prefer the official install script:- pip install uv + curl -LsSf https://astral.sh/uv/install.sh | sh
3 participants
Description
Added github action file e2e_test yaml file
Added docker compose and dockerfile related to the e2e tests
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Chores