test(e2e): add end-to-end tests for inference and CLI#780
Conversation
Signed-off-by: Dorin Geman <dorin.geman@docker.com>
| runs-on: macos-latest | ||
| timeout-minutes: 20 | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd | ||
| with: | ||
| submodules: recursive | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 | ||
| with: | ||
| go-version: 1.25.8 | ||
| cache: true | ||
|
|
||
| - name: Run e2e tests | ||
| run: make e2e |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
In general, the fix is to explicitly declare a permissions block that grants only the minimal required scopes to the GITHUB_TOKEN. Since this job only checks out repository contents and runs tests, it only needs read access to repository contents.
The best fix here is to add a permissions block to the e2e-test job (or at the workflow root). To keep the change tightly scoped and avoid affecting other workflows, we will add it at the job level, immediately under e2e-test: and aligned with runs-on:. We will set contents: read, which is sufficient for actions/checkout and normal test execution and preserves existing functionality while constraining token capabilities.
Concretely, in .github/workflows/e2e-test.yml, modify the e2e-test job definition so that:
- After line 11 (
e2e-test:), insert:permissions: contents: read
- Keep all existing steps and configuration unchanged.
No additional methods, imports, or definitions are needed.
| @@ -9,6 +9,8 @@ | ||
|
|
||
| jobs: | ||
| e2e-test: | ||
| permissions: | ||
| contents: read | ||
| runs-on: macos-latest | ||
| timeout-minutes: 20 | ||
|
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The GitHub workflow is configuring
go-version: 1.25.8, which is not a valid Go release; this should be updated to a real supported version (e.g., 1.22.x or 1.23.x) to avoid CI setup failures. make e2ealready depends onbuild-llamacppandbuild, butTestMainalso runsmake build, which is redundant and will slow down CI; consider relying on the pre-built binaries in CI or removing the extra build step fromTestMain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitHub workflow is configuring `go-version: 1.25.8`, which is not a valid Go release; this should be updated to a real supported version (e.g., 1.22.x or 1.23.x) to avoid CI setup failures.
- `make e2e` already depends on `build-llamacpp` and `build`, but `TestMain` also runs `make build`, which is redundant and will slow down CI; consider relying on the pre-built binaries in CI or removing the extra build step from `TestMain`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of end-to-end (e2e) tests for the model-runner. It adds new Makefile targets to facilitate building the llama.cpp submodule and executing these e2e tests. The tests cover both CLI functionality (pull, list, run, remove) and API-based inference (pull, list, chat, streaming, remove). A review comment highlighted an issue in the inference tests where io.ReadAll errors are ignored, which could mask underlying I/O problems and lead to confusing test failures.
Adds an end-to-end test suite that builds the full stack from source and runs inference and CLI tests against a live model-runner instance. The test harness follows the same pattern as the dmr convenience wrapper: find a free port, start the server pointing at the local llama.cpp build, wait for health, run tests, tear down.