Skip to content

docs: document running tests when Docker credential helper fails (e.g. WSL) #940

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.

Sign up for GitHub

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

Open
rich7420 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

docs: document running tests when Docker credential helper fails (e.g. WSL) #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,29 @@ ginkgo -v --focus "TestHandleAllocation" ./pkg/scheduler/actions/allocate
# Run tests with Ginkgo (for integration tests)
ginkgo -v --focus "test name pattern" ./pkg/binder/controllers/integration_tests

# Run tests with envtest (requires setup-envtest)
# Run tests with envtest (requires setup-envtest; K8s version from build/makefile/testenv.mk ENVTEST_K8S_VERSION)
make envtest
KUBEBUILDER_ASSETS="$(bin/setup-envtest use 1.34.0 -p path --bin-dir bin)" go test ./pkg/... -timeout 30m
```

#### Running tests when Docker is unavailable or credential helper fails

When Docker works, use `make test`. If it fails (e.g. WSL: `docker-credential-desktop.exe` not in PATH):

- **Fix Docker (WSL):** Backup and clear config, then `make test`. Restore for private registry: `cp ~/.docker/config.json.bak ~/.docker/config.json`.
```bash
cp ~/.docker/config.json ~/.docker/config.json.bak
echo '{}' > ~/.docker/config.json
```
Comment on lines +49 to +51
Copy link

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Risk: Docker config overwrite may lose other settings beyond credsStore.

The command echo '{}' > ~/.docker/config.json replaces the entire Docker configuration file, not just the credsStore field. Users may have other important settings in this file (e.g., auths for registry credentials, proxies, experimental flags, detachKeys, etc.) that will be lost.

🛡️ Safer alternative using jq 
-cp ~/.docker/config.json ~/.docker/config.json.bak
-echo '{}' > ~/.docker/config.json
+cp ~/.docker/config.json ~/.docker/config.json.bak
+jq 'del(.credsStore)' ~/.docker/config.json > ~/.docker/config.json.tmp && mv ~/.docker/config.json.tmp ~/.docker/config.json

This preserves other Docker settings while removing only the problematic credsStore field. Alternatively, document that users should manually edit the file to remove just the credsStore line, or verify they have no other important settings before using the destructive approach.

📝 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
cp ~/.docker/config.json ~/.docker/config.json.bak
echo '{}' > ~/.docker/config.json
```
cp ~/.docker/config.json ~/.docker/config.json.bak
jq 'del(.credsStore)' ~/.docker/config.json > ~/.docker/config.json.tmp && mv ~/.docker/config.json.tmp ~/.docker/config.json
🤖 Prompt for AI Agents 
In `@AGENTS.md` around lines 49 - 51, The current step that runs `echo '{}' >
~/.docker/config.json` dangerously overwrites the entire Docker config; instead
preserve other keys by updating the instruction to remove only the credsStore
field (keep the existing backup step `cp ~/.docker/config.json
~/.docker/config.json.bak`) and either: (a) use a safe JSON-editing tool (e.g.,
jq) to delete the top-level "credsStore" property and write the result back to
~/.docker/config.json, or (b) instruct users to manually remove the "credsStore"
line from their config and verify important keys like "auths", "proxies", and
"experimental" are retained; replace the destructive `echo '{}' >
~/.docker/config.json` line in AGENTS.md accordingly.

See [Docker Desktop WSL](https://docs.docker.com/desktop/wsl/).

- **Without Docker:** Unit: `go test ./pkg/... ./cmd/... -count=1 -short` (exit 1 if envtest packages fail). Integration: use `ENVTEST_K8S_VERSION` from `build/makefile/testenv.mk` (not `latest`):
```bash
make envtest
KUBEBUILDER_ASSETS="$(bin/setup-envtest use 1.34.0 -p path --bin-dir bin)" go test ./pkg/... -timeout 30m
```
Comment on lines +54 to +58
Copy link

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify "exit 1" expectation and consider version variable.

Two points:

  1. The phrase "exit 1 if envtest packages fail" may confuse users—why document a command expected to partially fail? Consider rephrasing to: "Unit tests (excludes envtest-dependent packages): go test ./pkg/... ./cmd/... -count=1 -short" or clarify that -short skips integration tests that require envtest.

  2. The example hardcodes version 1.34.0 even though line 54 instructs users to check ENVTEST_K8S_VERSION from build/makefile/testenv.mk. This could lead to version drift if the Makefile is updated but documentation is not. Consider using a placeholder like <VERSION> with a note to substitute the actual value, or accept the maintenance trade-off.

🤖 Prompt for AI Agents 
In `@AGENTS.md` around lines 54 - 58, Reword the unit test instruction to clarify
that the short test command excludes envtest-dependent packages (e.g., change to
"Unit tests (excludes envtest-dependent packages): go test ./pkg/... ./cmd/...
-count=1 -short") and replace the hardcoded "1.34.0" in the bin/setup-envtest
example with a placeholder or reference to the ENVTEST_K8S_VERSION value from
build/makefile/testenv.mk (e.g., use "<VERSION>" or a note to substitute
ENVTEST_K8S_VERSION) so the example uses the canonical version variable instead
of a literal; update the example invocation that calls bin/setup-envtest to
reflect this placeholder.

Full validation: `make validate`, `make lint`, unit tests, then envtest. Helm: `helm unittest` or CI.

#### E2E tests

E2E tests run against a real Kubernetes using [`kind`](https://kind.sigs.k8s.io/) cluster and are located in `test/e2e/suites/`.
Expand Down
39 changes: 38 additions & 1 deletion docs/developer/building-from-source.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,41 @@ To build and deploy KAI Scheduler from source, follow these steps:
5. Install on your cluster:
```sh
helm upgrade -i kai-scheduler -n kai-scheduler --create-namespace ./charts/kai-scheduler-0.0.0.tgz
```
```

## Testing

From the repository root, run `make test` (Helm chart + Go tests in Docker).

### When `make test` fails (e.g. WSL: `docker-credential-desktop.exe` not in PATH)

**Option 1 — Fix Docker config (WSL):**

```sh
cp ~/.docker/config.json ~/.docker/config.json.bak
echo '{}' > ~/.docker/config.json
make test
```

Restore for private registry: `cp ~/.docker/config.json.bak ~/.docker/config.json`. Docker Desktop may restore `credsStore` on restart. See [Docker Desktop WSL](https://docs.docker.com/desktop/wsl/).

**Option 2 — Run without Docker:**

- Unit tests: `go test ./pkg/... ./cmd/... -count=1 -short` (envtest-dependent packages may fail; exit 1 is expected).
- Integration (envtest): use `ENVTEST_K8S_VERSION` from `build/makefile/testenv.mk` (e.g. 1.34.0; do not use `latest`):
```sh
make envtest
KUBEBUILDER_ASSETS="$(bin/setup-envtest use 1.34.0 -p path --bin-dir bin)" go test ./pkg/... -timeout 30m
Comment on lines +54 to +58
Copy link

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify unit test expectations and version hardcoding.

Two minor points:

  1. "exit 1 is expected": This phrasing suggests the command will fail, which may confuse users. Consider clarifying that -short skips integration tests requiring envtest, so some packages won't be tested: "Unit tests only (skips envtest integration tests): go test ./pkg/... ./cmd/... -count=1 -short".

  2. Hardcoded version 1.34.0: Line 55 directs users to build/makefile/testenv.mk for ENVTEST_K8S_VERSION, yet the example hardcodes 1.34.0. If the Makefile is updated, this documentation will become stale. Consider using <VERSION> as a placeholder, or accept the maintenance cost for the sake of a working copy-paste example.

🤖 Prompt for AI Agents 
In `@docs/developer/building-from-source.md` around lines 54 - 58, Update the unit
test line to clarify intent by explaining it runs only unit tests and skips
envtest-dependent integration tests (e.g., change the phrasing around `go test
./pkg/... ./cmd/... -count=1 -short` to say "Unit tests only (skips envtest
integration tests)"). Also replace the hardcoded `1.34.0` in the envtest example
with a placeholder like `<ENVTEST_K8S_VERSION>` and add a short note to fetch
the actual version from `ENVTEST_K8S_VERSION` in `build/makefile/testenv.mk`
when running the `make envtest` / `bin/setup-envtest` commands so the docs stay
correct if the Makefile version changes.

```
- Validation: `make validate`, `make lint`.

### Version reference

| What | Defined in | Example |
|------|------------|---------|
| envtest K8s | `build/makefile/testenv.mk` → `ENVTEST_K8S_VERSION` | 1.34.0 |
| envtest tool | `build/makefile/testenv.mk` → `ENVTEST_VERSION` | release-0.20 |
| Go / builder | `build/makefile/golang.mk`, `build/builder/Dockerfile` | 1.24.4-bullseye |

Use Makefile/Dockerfile values; do not use `latest`.