Skip to content

Commit 186d322

Browse files
committed
test: fix pre-commit issues in e2e test framework
- Add license headers to all Python and shell files - Fix ruff import ordering and formatting - Fix shellcheck warnings (brace variable references) - Remove hard line wraps in README and scenarios prose Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
1 parent bb5fc7e commit 186d322

8 files changed

Lines changed: 131 additions & 148 deletions

File tree

tests/e2e/README.md

Lines changed: 21 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -34,60 +34,37 @@ Behavioral end-to-end tests for rocm-cli using Robot Framework.
3434
└─────────────────┘
3535
```
3636

37-
Tests run the real `rocm` binary via subprocess, the same way a user would.
38-
Keyword libraries parse CLI output and make HTTP requests to served endpoints.
39-
No imports from the rocm-cli codebase.
37+
Tests run the real `rocm` binary via subprocess, the same way a user would. Keyword libraries parse CLI output and make HTTP requests to served endpoints. No imports from the rocm-cli codebase.
4038

4139
## Why a separate test layer
4240

4341
The existing test infrastructure covers two levels:
4442

45-
- **Rust unit and integration tests** (`cargo test --workspace`) — pure,
46-
deterministic, no GPU or runtime needed. These validate individual components
47-
in isolation with fixtures and mocks.
48-
- **Python smoke scripts** (`scripts/smoke_local.py`,
49-
`scripts/*_therock_gpu_test.py`) — zero-dependency, stdlib-only scripts that
50-
verify CLI behavior against real or fake binaries. These run as linear scripts
51-
with basic substring assertions.
43+
- **Rust unit and integration tests** (`cargo test --workspace`) — pure, deterministic, no GPU or runtime needed. These validate individual components in isolation with fixtures and mocks.
44+
- **Python smoke scripts** (`scripts/smoke_local.py`, `scripts/*_therock_gpu_test.py`) — zero-dependency, stdlib-only scripts that verify CLI behavior against real or fake binaries. These run as linear scripts with basic substring assertions.
5245

53-
Both layers are valuable but leave a gap: **cross-component user journeys are
54-
not tested.** Each component works individually, but the seams between them
55-
break in practice. For example:
46+
Both layers are valuable but leave a gap: **cross-component user journeys are not tested.** Each component works individually, but the seams between them break in practice. For example:
5647

57-
- Model alias resolution works in the recipe registry but is not forwarded
58-
correctly to all engine adapters.
59-
- `rocm serve` binds to port 11435 but the TUI chat detection probes ports
60-
8000 and 13305.
61-
- The managed-services registry tracks a running server with its real model
62-
name, but the TUI chat falls back to a hardcoded `local-model` default.
48+
- Model alias resolution works in the recipe registry but is not forwarded correctly to all engine adapters.
49+
- `rocm serve` binds to port 11435 but the TUI chat detection probes ports 8000 and 13305.
50+
- The managed-services registry tracks a running server with its real model name, but the TUI chat falls back to a hardcoded `local-model` default.
6351

64-
These are integration failures that only surface when exercising the full user
65-
flow: install → examine → configure runtime → serve model → detect → chat.
52+
These are integration failures that only surface when exercising the full user flow: install → examine → configure runtime → serve model → detect → chat.
6653

6754
## Why Robot Framework
6855

69-
The product scripts (`scripts/*.py`) are deliberately zero-dependency to run on
70-
user machines and in minimal CI environments. E2E tests have a different
71-
audience — they run in developer and CI environments where installing packages
72-
is expected. Keeping the two concerns separate avoids compromising either:
56+
The product scripts (`scripts/*.py`) are deliberately zero-dependency to run on user machines and in minimal CI environments. E2E tests have a different audience — they run in developer and CI environments where installing packages is expected. Keeping the two concerns separate avoids compromising either:
7357

7458
- Product scripts stay zero-dependency and stdlib-only.
75-
- E2E tests get proper test infrastructure without reinventing scenario
76-
discovery, tagging, parameterization, setup/teardown lifecycle, and
77-
structured reporting.
59+
- E2E tests get proper test infrastructure without reinventing scenario discovery, tagging, parameterization, setup/teardown lifecycle, and structured reporting.
7860

7961
Robot Framework was chosen because:
8062

81-
- **Keyword-driven syntax**`.robot` files are readable without knowing
82-
Python, making scenarios accessible to anyone reviewing test coverage.
83-
- **Built-in tagging** — scenarios can be tagged `gpu`, `mock`, `smoke`, etc.
84-
and selectively run in different CI environments.
85-
- **Setup/teardown lifecycle** — suite and test-level setup and teardown handle
86-
isolated config directories, server startup/shutdown, and cleanup.
87-
- **Structured reporting** — HTML reports show which scenario failed, at which
88-
step, with full output, without custom reporting code.
89-
- **Step reuse** — shared keywords across scenarios without building a custom
90-
framework.
63+
- **Keyword-driven syntax**`.robot` files are readable without knowing Python, making scenarios accessible to anyone reviewing test coverage.
64+
- **Built-in tagging** — scenarios can be tagged `gpu`, `mock`, `smoke`, etc. and selectively run in different CI environments.
65+
- **Setup/teardown lifecycle** — suite and test-level setup and teardown handle isolated config directories, server startup/shutdown, and cleanup.
66+
- **Structured reporting** — HTML reports show which scenario failed, at which step, with full output, without custom reporting code.
67+
- **Step reuse** — shared keywords across scenarios without building a custom framework.
9168

9269
## Directory layout
9370

@@ -175,9 +152,7 @@ python3 -m robot --pythonpath tests/e2e --outputdir tests/e2e/results tests/e2e/
175152

176153
## From scenarios to tests
177154

178-
Scenarios are written in BDD style (Given/When/Then). The `.robot` file is a
179-
near-verbatim copy of the behavioral scenario. The keyword library translates
180-
the behavioral language into technical actions.
155+
Scenarios are written in BDD style (Given/When/Then). The `.robot` file is a near-verbatim copy of the behavioral scenario. The keyword library translates the behavioral language into technical actions.
181156

182157
```
183158
Scenario (spec) → .robot file (same words) → Keyword Library (Python)
@@ -226,23 +201,10 @@ def user_serves_model(self, alias: str, engine: str):
226201

227202
## Test design principles
228203

229-
- **Platform-agnostic scenarios.** The same behavioral expectations apply
230-
across MI300X, RDNA 3, and WSL. Hardware determines which GPU is detected or
231-
which engine is auto-selected, but the contract is the same: "examine detects
232-
a GPU," "serve makes a model reachable," "chat connects and gets a response."
233-
234-
- **Real binary, real hardware.** Tests run the actual `rocm` binary, not a
235-
mock. On GPU runners they exercise real serving and inference. On CPU-only
236-
runners they verify graceful failure and non-GPU paths.
237-
238-
- **Isolated state.** Each suite uses isolated `ROCM_CLI_CONFIG_DIR`,
239-
`ROCM_CLI_DATA_DIR`, and `ROCM_CLI_CACHE_DIR` directories (following the
240-
pattern established by `smoke_local.py`). Tests never touch the user's real
241-
`~/.rocm` state.
242-
243-
- **Black-box only.** Keywords interact with `rocm` through its CLI and HTTP
244-
endpoints. No imports from the rocm-cli Rust codebase or internal Python
245-
modules. Tests verify observable behavior, not implementation details.
204+
- **Platform-agnostic scenarios.** The same behavioral expectations apply across MI300X, RDNA 3, and WSL. Hardware determines which GPU is detected or which engine is auto-selected, but the contract is the same: "examine detects a GPU," "serve makes a model reachable," "chat connects and gets a response."
205+
- **Real binary, real hardware.** Tests run the actual `rocm` binary, not a mock. On GPU runners they exercise real serving and inference. On CPU-only runners they verify graceful failure and non-GPU paths.
206+
- **Isolated state.** Each suite uses isolated `ROCM_CLI_CONFIG_DIR`, `ROCM_CLI_DATA_DIR`, and `ROCM_CLI_CACHE_DIR` directories (following the pattern established by `smoke_local.py`). Tests never touch the user's real `~/.rocm` state.
207+
- **Black-box only.** Keywords interact with `rocm` through its CLI and HTTP endpoints. No imports from the rocm-cli Rust codebase or internal Python modules. Tests verify observable behavior, not implementation details.
246208

247209
## Relationship to existing tests
248210

@@ -255,8 +217,7 @@ This layer does **not** replace existing tests:
255217
| `*_therock_gpu_test.py` | Per-engine GPU validation | Hardware CI (planned) |
256218
| **`tests/e2e/`** | Cross-component user journeys | Hardware CI + mock subset on hosted CI |
257219

258-
The e2e suites are the only layer that tests the seams between components —
259-
where most user-facing bugs live.
220+
The e2e suites are the only layer that tests the seams between components — where most user-facing bugs live.
260221

261222
## Writing new tests
262223

tests/e2e/libraries/RocmCliLibrary.py

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1+
# Copyright Advanced Micro Devices, Inc.
2+
#
3+
# SPDX-License-Identifier: MIT
4+
15
"""Keyword library for core rocm-cli interactions.
26
37
Maps BDD step text to CLI commands and output assertions. Covers
48
examine, runtime, engine, and version scenarios.
59
"""
610

11+
from common.cli_runner import CliResult, run_rocm
712
from robot.api import logger
813
from robot.api.deco import keyword, library
914

10-
from common.cli_runner import CliResult, run_rocm
11-
1215

1316
@library(scope="TEST")
1417
class RocmCliLibrary:
@@ -28,9 +31,9 @@ def __init__(self):
2831
def setup_gpu_machine(self):
2932
"""Precondition: verify GPU is detectable (will fail on CPU-only)."""
3033
result = run_rocm("examine")
31-
assert result.contains("AMD GPU detected") or result.contains("detected_gfx_target"), (
32-
f"No AMD GPU detected on this machine:\n{result.stdout}"
33-
)
34+
assert result.contains("AMD GPU detected") or result.contains(
35+
"detected_gfx_target"
36+
), f"No AMD GPU detected on this machine:\n{result.stdout}"
3437

3538
@keyword("a machine with a pre-installed ROCm that was not set up by the CLI")
3639
def setup_unmanaged_rocm(self):
@@ -41,9 +44,9 @@ def setup_unmanaged_rocm(self):
4144
def setup_no_runtimes(self):
4245
"""Precondition: verify no managed runtimes are registered."""
4346
result = run_rocm("runtimes", "list")
44-
assert result.contains("installed: none") or result.contains("managed_runtimes: 0"), (
45-
f"Expected no managed runtimes:\n{result.stdout}"
46-
)
47+
assert result.contains("installed: none") or result.contains(
48+
"managed_runtimes: 0"
49+
), f"Expected no managed runtimes:\n{result.stdout}"
4750

4851
@keyword("a machine with a standard ROCm install")
4952
def setup_standard_rocm(self):
@@ -91,9 +94,12 @@ def user_installs_sdk(self):
9194
def user_tries_adopt(self):
9295
"""Run rocm runtimes adopt with /opt/rocm."""
9396
self._result = run_rocm(
94-
"runtimes", "adopt",
95-
"--python", "/usr/bin/python3",
96-
"--root", "/opt/rocm",
97+
"runtimes",
98+
"adopt",
99+
"--python",
100+
"/usr/bin/python3",
101+
"--root",
102+
"/opt/rocm",
97103
)
98104
logger.info(self._result.stdout)
99105

@@ -153,17 +159,17 @@ def assert_gpu_detected(self):
153159
def assert_driver_available(self):
154160
"""Assert examine output shows driver available."""
155161
assert self._result is not None, "No command was run"
156-
assert self._result.contains("amdgpu") or self._result.contains("driver_status"), (
157-
f"Driver status not found in examine output:\n{self._result.stdout}"
158-
)
162+
assert self._result.contains("amdgpu") or self._result.contains(
163+
"driver_status"
164+
), f"Driver status not found in examine output:\n{self._result.stdout}"
159165

160166
@keyword("the inspection reports the ROCm install as unmanaged")
161167
def assert_rocm_unmanaged(self):
162168
"""Assert examine shows legacy/unmanaged ROCm."""
163169
assert self._result is not None, "No command was run"
164-
assert self._result.contains("detected_unmanaged") or self._result.contains("legacy"), (
165-
f"Expected unmanaged ROCm status:\n{self._result.stdout}"
166-
)
170+
assert self._result.contains("detected_unmanaged") or self._result.contains(
171+
"legacy"
172+
), f"Expected unmanaged ROCm status:\n{self._result.stdout}"
167173

168174
@keyword("the inspection suggests installing a managed runtime")
169175
def assert_suggests_managed_runtime(self):
@@ -211,9 +217,9 @@ def assert_adopt_error_explains(self):
211217
"""Assert error message mentions TheRock or unsupported."""
212218
assert self._result is not None, "No command was run"
213219
output = self._result.stdout + self._result.stderr
214-
assert any(term in output.lower() for term in ("therock", "rocm_sdk", "not supported")), (
215-
f"Error does not explain TheRock requirement:\n{output}"
216-
)
220+
assert any(
221+
term in output.lower() for term in ("therock", "rocm_sdk", "not supported")
222+
), f"Error does not explain TheRock requirement:\n{output}"
217223

218224
@keyword("the serve plan shows the full model identifier")
219225
def assert_serve_plan_full_id(self):

tests/e2e/libraries/ServeLibrary.py

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# Copyright Advanced Micro Devices, Inc.
2+
#
3+
# SPDX-License-Identifier: MIT
4+
15
"""Keyword library for model serving, endpoint interaction, and chat.
26
37
Manages the mock OpenAI server for mock-tier tests and interacts
@@ -9,11 +13,10 @@
913
import json
1014
import urllib.request
1115

12-
from robot.api import logger
13-
from robot.api.deco import keyword, library
14-
1516
from common.cli_runner import CliResult, run_rocm
1617
from common.mock_server import MockOpenAIServer
18+
from robot.api import logger
19+
from robot.api.deco import keyword, library
1720

1821

1922
@library(scope="TEST")
@@ -89,8 +92,11 @@ def setup_localhost_model(self):
8992
def setup_gpu_model(self):
9093
"""Start a real model server using rocm serve."""
9194
self._serve_result = run_rocm(
92-
"serve", "Qwen/Qwen2.5-1.5B-Instruct",
93-
"--engine", "vllm", "--managed",
95+
"serve",
96+
"Qwen/Qwen2.5-1.5B-Instruct",
97+
"--engine",
98+
"vllm",
99+
"--managed",
94100
timeout=600,
95101
)
96102
assert self._serve_result.rc == 0, (
@@ -126,10 +132,12 @@ def send_chat_via_cli(self):
126132
"""Send a chat completion request to the endpoint."""
127133
assert self._endpoint is not None, "No endpoint configured"
128134
url = f"{self._endpoint}/chat/completions"
129-
payload = json.dumps({
130-
"model": self._model_name or "local-model",
131-
"messages": [{"role": "user", "content": "Hello"}],
132-
}).encode()
135+
payload = json.dumps(
136+
{
137+
"model": self._model_name or "local-model",
138+
"messages": [{"role": "user", "content": "Hello"}],
139+
}
140+
).encode()
133141
req = urllib.request.Request(
134142
url,
135143
data=payload,
@@ -153,7 +161,9 @@ def user_sends_chat(self):
153161
def user_serves_default_engine(self):
154162
"""Run rocm serve with a small model, no --engine flag."""
155163
self._serve_result = run_rocm(
156-
"serve", "Qwen/Qwen2.5-1.5B-Instruct", "--managed",
164+
"serve",
165+
"Qwen/Qwen2.5-1.5B-Instruct",
166+
"--managed",
157167
timeout=600,
158168
)
159169
logger.info(self._serve_result.stdout)
@@ -270,9 +280,7 @@ def assert_model_reachable(self):
270280
req = urllib.request.Request(url)
271281
with urllib.request.urlopen(req, timeout=10) as resp:
272282
data = json.loads(resp.read())
273-
assert "data" in data and len(data["data"]) > 0, (
274-
f"No models at {url}: {data}"
275-
)
283+
assert "data" in data and len(data["data"]) > 0, f"No models at {url}: {data}"
276284

277285
@keyword("the endpoint responds to inference requests")
278286
def assert_endpoint_responds(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Copyright Advanced Micro Devices, Inc.
2+
#
3+
# SPDX-License-Identifier: MIT

tests/e2e/libraries/common/cli_runner.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# Copyright Advanced Micro Devices, Inc.
2+
#
3+
# SPDX-License-Identifier: MIT
4+
15
"""Subprocess wrapper for running the rocm binary."""
26

37
from __future__ import annotations

tests/e2e/libraries/common/mock_server.py

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# Copyright Advanced Micro Devices, Inc.
2+
#
3+
# SPDX-License-Identifier: MIT
4+
15
"""Minimal mock OpenAI-compatible server for E2E tests.
26
37
Implements GET /v1/models and POST /v1/chat/completions with canned
@@ -15,10 +19,12 @@
1519
class MockOpenAIHandler(BaseHTTPRequestHandler):
1620
def do_GET(self):
1721
if self.path.rstrip("/") in ("/v1/models", "/models"):
18-
body = json.dumps({
19-
"object": "list",
20-
"data": [{"id": self.server.model_name, "object": "model"}],
21-
})
22+
body = json.dumps(
23+
{
24+
"object": "list",
25+
"data": [{"id": self.server.model_name, "object": "model"}],
26+
}
27+
)
2228
self._respond(200, body)
2329
else:
2430
self._respond(404, json.dumps({"error": "not found"}))
@@ -31,24 +37,28 @@ def do_POST(self):
3137
received_model = request.get("model", "<missing>")
3238
self.server.received_models.append(received_model)
3339

34-
body = json.dumps({
35-
"id": "mock-completion-1",
36-
"object": "chat.completion",
37-
"model": received_model,
38-
"choices": [{
39-
"index": 0,
40-
"message": {
41-
"role": "assistant",
42-
"content": "This is a mock response for testing.",
40+
body = json.dumps(
41+
{
42+
"id": "mock-completion-1",
43+
"object": "chat.completion",
44+
"model": received_model,
45+
"choices": [
46+
{
47+
"index": 0,
48+
"message": {
49+
"role": "assistant",
50+
"content": "This is a mock response for testing.",
51+
},
52+
"finish_reason": "stop",
53+
}
54+
],
55+
"usage": {
56+
"prompt_tokens": 10,
57+
"completion_tokens": 8,
58+
"total_tokens": 18,
4359
},
44-
"finish_reason": "stop",
45-
}],
46-
"usage": {
47-
"prompt_tokens": 10,
48-
"completion_tokens": 8,
49-
"total_tokens": 18,
50-
},
51-
})
60+
}
61+
)
5262
self._respond(200, body)
5363
else:
5464
self._respond(404, json.dumps({"error": "not found"}))
@@ -66,8 +76,12 @@ def log_message(self, format, *args):
6676
class MockOpenAIServer:
6777
"""Start and stop a mock OpenAI-compatible server in a background thread."""
6878

69-
def __init__(self, host: str = "127.0.0.1", port: int = 18435,
70-
model_name: str = "TestModel/E2E-1B"):
79+
def __init__(
80+
self,
81+
host: str = "127.0.0.1",
82+
port: int = 18435,
83+
model_name: str = "TestModel/E2E-1B",
84+
):
7185
self.host = host
7286
self.port = port
7387
self.model_name = model_name

0 commit comments

Comments
 (0)