From 842e18d6bc8dda4326682006fcf1c638347fd7e1 Mon Sep 17 00:00:00 2001 From: wellCh4n Date: Sat, 5 Sep 2026 02:00:03 +0800 Subject: [PATCH 1/2] fix(api): let SSE responses finish cleanly and stay quiet when the viewer leaves Two things went wrong at the end of every server-sent event stream, and neither was visible from a browser because an EventSource closes itself on the `end` event and reconnects on anything else. The completion of an SseEmitter is an ASYNC dispatch that walks the security filter chain again. JwtAuthFilter is a OncePerRequestFilter and sits that dispatch out, so nothing is authenticated by then and the "/api/**" rule denied a request it had already let through. With the response committed the only effect was Tomcat cutting the connection before the final chunk, plus an AuthorizationDeniedException stack trace per stream; a client reading to the end saw "Response ended prematurely". The ASYNC dispatch is only ever reached through an authorized REQUEST dispatch, so it is now permitted outright, which is what Spring Security recommends. A browser closing an EventSource mid-stream reaches the DispatcherServlet as the container's error notification, an AsyncRequestNotUsableException. The catch-all in GlobalExceptionHandler logged it as an unhandled error and then failed again trying to write a JSON envelope onto a text/event-stream response, two stack traces for someone leaving the pipeline page. A dedicated handler now logs it at debug and returns nothing: nobody is listening. Co-Authored-By: Claude Fable 5.1 --- .../config/SecurityConfiguration.java | 9 +++++++++ .../interfaces/rest/GlobalExceptionHandler.java | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/main/java/com/github/wellch4n/oops/infrastructure/config/SecurityConfiguration.java b/src/main/java/com/github/wellch4n/oops/infrastructure/config/SecurityConfiguration.java index acea78bc..549aef51 100644 --- a/src/main/java/com/github/wellch4n/oops/infrastructure/config/SecurityConfiguration.java +++ b/src/main/java/com/github/wellch4n/oops/infrastructure/config/SecurityConfiguration.java @@ -1,5 +1,6 @@ package com.github.wellch4n.oops.infrastructure.config; +import jakarta.servlet.DispatcherType; import jakarta.servlet.http.HttpServletResponse; import java.util.List; import org.springframework.context.annotation.Bean; @@ -43,6 +44,14 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { response.sendError(HttpServletResponse.SC_FORBIDDEN)) ) .authorizeHttpRequests(auth -> auth + // An SSE response (pipeline logs, application events) completes on an + // ASYNC dispatch that walks this chain again. JwtAuthFilter, like any + // OncePerRequestFilter, sits that dispatch out, so nothing is + // authenticated by then and "/api/**" would deny a request it already + // let through — with the response committed, the only effect is the + // connection being cut before the final chunk. The ASYNC dispatch is + // only ever reached through an authorized REQUEST dispatch. + .dispatcherTypeMatchers(DispatcherType.ASYNC).permitAll() .requestMatchers("/api/auth/**").permitAll() .requestMatchers("/api/auth/feishu/**").permitAll() .requestMatchers("/api/health").permitAll() diff --git a/src/main/java/com/github/wellch4n/oops/interfaces/rest/GlobalExceptionHandler.java b/src/main/java/com/github/wellch4n/oops/interfaces/rest/GlobalExceptionHandler.java index 3284aa92..43082d18 100644 --- a/src/main/java/com/github/wellch4n/oops/interfaces/rest/GlobalExceptionHandler.java +++ b/src/main/java/com/github/wellch4n/oops/interfaces/rest/GlobalExceptionHandler.java @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.async.AsyncRequestNotUsableException; @RestControllerAdvice public class GlobalExceptionHandler { @@ -17,6 +18,18 @@ public Result handleBizException(BizException exception) { return Result.failure(exception.getMessage()); } + /** + * A browser closing an {@code EventSource} mid-stream — leaving the pipeline page, switching to + * another step's log — reaches here as the container's error dispatch for the SSE request. + * Nothing is wrong and nobody is listening: the catch-all below would log it as an error and + * then fail again trying to write a JSON envelope onto a {@code text/event-stream} response. + * Returning nothing marks the response as handled, which for a dead connection is exactly right. + */ + @ExceptionHandler(AsyncRequestNotUsableException.class) + public void handleDisconnectedClient(AsyncRequestNotUsableException exception) { + log.debug("Client disconnected from a streaming response: {}", exception.getMessage()); + } + @ExceptionHandler(Exception.class) public Result handleException(Exception exception) { log.error("Unhandled exception", exception); From 12b17b108aa5782f8cf35932cee8e11c7a0066ef Mon Sep 17 00:00:00 2001 From: wellCh4n Date: Sat, 5 Sep 2026 02:00:03 +0800 Subject: [PATCH 2/2] test(integration): cover the pipeline SSE streams and fix the sockets that passed vacuously The pipeline log moved from one WebSocket to two server-sent event streams, which left the route inventory stale and the streams module connecting to a handler that no longer exists. routes.json is regenerated, and the client gains an SSE reader that records the GET for the coverage check. The streams module now pins the SSE contract: the status stream opens with `steps`, ends with `end`, and its final snapshot is a Succeeded phase with every step SUCCEEDED; a step's log replays in stamped batches whose event id is the last line's time; resuming with Last-Event-ID after the final line replays nothing; an unknown step answers `error` then `end`. The old status/watch test in operations uses the same reader instead of a hand-rolled request. The pod log and terminal socket tests never passed `?environment=`, so the server closed the socket at once and the tests proved nothing. Passing it exposed a hang in read_until_closed: recv() answers the server's ten-second protocol pings itself and restarts its timeout on each, so an idle socket never timed out. Frames are now read one at a time against a wall-clock deadline, with pings answered here. The ping/pong test moves to the pod log socket, which is the log socket that remains. The coverage figure is recorded on the test suite rather than the case, which is what the xunit2 JUnit schema allows. AGENTS.md named the build steps clone/build/push; the containers, and therefore the `steps` event, are fetch/compile/publish, which is what the suite asserts. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 8 +- tests/integration/README.md | 4 +- tests/integration/oops_client.py | 100 +++++++++++++-- tests/integration/routes.json | 12 ++ tests/integration/test_operations.py | 41 +++--- tests/integration/test_streams.py | 175 ++++++++++++++++++++------ tests/integration/test_zz_coverage.py | 15 ++- 7 files changed, 272 insertions(+), 83 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3538904b..be764e5d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -164,11 +164,11 @@ alike (`?environment={name}`, never `?env=`). Nothing outside the `environment` environment ids. ### Pipelines -Build pipelines run as Kubernetes Jobs with init containers: -1. **clone**: Clones source code from git (or downloads ZIP via curl for ZIP source type) +Build pipelines run as Kubernetes Jobs with init containers. The container names are what the step bar and the `steps` SSE event show, so they are part of the contract the integration suite pins — `fetch`, `dockerfile`, `compile`, `publish`, then the trailing `done` main container: +1. **fetch** (`CloneContainer`): Clones source code from git (or downloads ZIP via curl for ZIP source type) 2. **dockerfile** (optional): Writes an inline `USER` Dockerfile into the workspace -3. **build** (optional): Runs custom build commands -4. **push**: Builds and pushes Docker image using Buildah, with the `overlay` storage driver over the `container-storage` emptyDir that `ContainerStorageVolume` mounts at `/var/lib/containers`. Both halves matter: the store defaults into the container's writable layer, where every write pays an overlayfs copy-up, and `vfs` (used until the volume existed) copies the entire tree per layer — together they made a build of any size stall for minutes with no log output after its last line. Buildah's own image declares a `VOLUME` for this path, but Kubernetes ignores image `VOLUME` declarations, so the mount has to be explicit. +3. **compile** (optional, `CompileContainer`): Runs custom build commands +4. **publish** (`PublishContainer`): Builds and pushes Docker image using Buildah, with the `overlay` storage driver over the `container-storage` emptyDir that `ContainerStorageVolume` mounts at `/var/lib/containers`. Both halves matter: the store defaults into the container's writable layer, where every write pays an overlayfs copy-up, and `vfs` (used until the volume existed) copies the entire tree per layer — together they made a build of any size stall for minutes with no log output after its last line. Buildah's own image declares a `VOLUME` for this path, but Kubernetes ignores image `VOLUME` declarations, so the mount has to be explicit. Two source types exist: `GIT` (default) and `ZIP`. ZIP uploads use presigned S3 URLs via `BuildSourceObjectStorageService` — the frontend gets a presigned PUT URL from `POST .../deployments/source-upload`, uploads the file, then triggers the pipeline. ZIP builds use `oops.pipeline.image.zip` (defaults to `alpine/curl:8.17.0`) to download the archive. diff --git a/tests/integration/README.md b/tests/integration/README.md index 8956eb5a..4bf99760 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -1,6 +1,6 @@ # Integration suite -Black-box tests against the OOPS HTTP and WebSocket API. They import nothing from +Black-box tests against the OOPS HTTP, SSE and WebSocket API. They import nothing from the backend and know nothing about its language, so the same suite is the acceptance criteria for the current Java service, a Go or Rust rewrite, or a GraalVM native build. @@ -15,7 +15,7 @@ observable behaviour while a correct implementation still exists to check agains | Tier | Needs | Covers | |---|---|---| | contract | the compose middleware | the wire contract, users and authorisation, domains, application configuration round trips, environment management, object storage presigning, git branch listing | -| `cluster` | the above plus k3s | build and deploy pipelines, pipeline operations, both registry credential paths, git clone with and without credentials, ingress routes and basic auth, deletion in the cluster, sandboxes, pod filesystem, log and terminal sockets | +| `cluster` | the above plus k3s | build and deploy pipelines, pipeline operations, both registry credential paths, git clone with and without credentials, ingress routes and basic auth, deletion in the cluster, sandboxes, pod filesystem, pipeline step and log event streams, pod log and terminal sockets | Organised by scenario rather than by endpoint: what a rewrite has to preserve is behaviour, and every defect this suite found needed a full journey to surface. diff --git a/tests/integration/oops_client.py b/tests/integration/oops_client.py index 5f04d568..2686a01e 100644 --- a/tests/integration/oops_client.py +++ b/tests/integration/oops_client.py @@ -1,4 +1,4 @@ -"""HTTP and WebSocket client for the OOPS API. +"""HTTP, WebSocket and server-sent event client for the OOPS API. Deliberately knows nothing about the backend implementation — it speaks only the wire protocol, so the same suite validates the current Java service, a Go or Rust @@ -246,6 +246,73 @@ def websocket(self, path: str, timeout: int = 60) -> websocket.WebSocket: connection = websocket.create_connection(url, timeout=timeout) return connection + # -- server-sent events ------------------------------------------------- + + def sse(self, path: str, timeout: int = 60, + headers: Optional[dict] = None) -> Iterator[SseEvent]: + """Open an event stream and yield its events until the server ends the + response or `timeout` seconds pass with nothing arriving. + + A GET like any other, so it is recorded as one for coverage. `headers` + is for what a browser's EventSource sends on its own — `Last-Event-ID` + on a reconnect — since that is the only input these streams take beyond + the URL. + """ + record("GET", path) + request_headers = {"Accept": "text/event-stream"} + if self.token: + request_headers["Authorization"] = f"Bearer {self.token}" + if headers: + request_headers.update(headers) + response = self.session.get(self.endpoint + path, headers=request_headers, + stream=True, timeout=timeout) + try: + if response.status_code != 200: + raise ApiError( + f"GET {path} returned {response.status_code} instead of an " + f"event stream: {response.text[:400]!r}") + content_type = response.headers.get("Content-Type", "") + if not content_type.startswith("text/event-stream"): + raise ApiError( + f"GET {path} answered with {content_type!r}, not text/event-stream") + yield from parse_sse(response.iter_lines()) + finally: + response.close() + + +@dataclass +class SseEvent: + """One server-sent event: the `event:` name, the `id:` if the server set one, + and the `data:` lines joined back together.""" + + event: str + id: Optional[str] + data: str + + +def parse_sse(lines: Iterator[bytes]) -> Iterator[SseEvent]: + """Decode the wire format: `field:value` lines, a blank line ends an event, + a leading `:` is a comment. Yields events in arrival order.""" + event, event_id, data = "message", None, [] + for raw in lines: + line = raw.decode("utf-8", errors="replace") + if line == "": + if data or event != "message": + yield SseEvent(event, event_id, "\n".join(data)) + event, event_id, data = "message", None, [] + continue + if line.startswith(":"): + continue + field, _, value = line.partition(":") + if value.startswith(" "): + value = value[1:] + if field == "event": + event = value + elif field == "id": + event_id = value + elif field == "data": + data.append(value) + def wait_until(predicate: Callable[[], Any], timeout: int = 300, interval: float = 3.0, description: str = "condition") -> Any: @@ -279,17 +346,36 @@ def read_until_closed(connection: websocket.WebSocket, Log sockets answer a text "ping" with "pong"; terminal sockets must not be pinged, because their stdin is live. + + Frames are read one at a time rather than through `recv()`, which swallows + control frames and waits on for a data frame with a fresh timeout each time + — and every handler sends a protocol-level ping every few seconds, so an + idle socket would never time out. The pings are answered here instead, and + the deadline is the wall clock. """ deadline = time.time() + timeout - while time.time() < deadline: + while True: + remaining = deadline - time.time() + if remaining <= 0: + return + connection.settimeout(remaining) try: - frame = connection.recv() + frame = connection.recv_frame() except websocket.WebSocketTimeoutException: return except websocket.WebSocketConnectionClosedException: return - if frame is None or frame == "": + if frame is None or frame.opcode == websocket.ABNF.OPCODE_CLOSE: return - if isinstance(frame, bytes): - frame = frame.decode("utf-8", errors="replace") - yield frame + if frame.opcode == websocket.ABNF.OPCODE_PING: + connection.pong(frame.data) + continue + if frame.opcode not in (websocket.ABNF.OPCODE_TEXT, + websocket.ABNF.OPCODE_BINARY): + continue + data = frame.data + if isinstance(data, bytes): + data = data.decode("utf-8", errors="replace") + if data == "": + continue + yield data diff --git a/tests/integration/routes.json b/tests/integration/routes.json index 94501f88..1bcabdeb 100644 --- a/tests/integration/routes.json +++ b/tests/integration/routes.json @@ -227,6 +227,18 @@ "key": "GET /api/namespaces/{}/applications/{}/pipelines/{}", "controller": "PipelineController" }, + { + "method": "GET", + "path": "/api/namespaces/{namespace}/applications/{name}/pipelines/{id}/log", + "key": "GET /api/namespaces/{}/applications/{}/pipelines/{}/log", + "controller": "PipelineController" + }, + { + "method": "GET", + "path": "/api/namespaces/{namespace}/applications/{name}/pipelines/{id}/steps/watch", + "key": "GET /api/namespaces/{}/applications/{}/pipelines/{}/steps/watch", + "controller": "PipelineController" + }, { "method": "GET", "path": "/api/namespaces/{namespace}/applications/{name}/pods/{pod}/files", diff --git a/tests/integration/test_operations.py b/tests/integration/test_operations.py index 97303d4c..c5c25f66 100644 --- a/tests/integration/test_operations.py +++ b/tests/integration/test_operations.py @@ -60,36 +60,23 @@ def test_pipelines_can_be_searched_for_an_application(client, namespace, @pytest.mark.cluster def test_the_status_stream_opens_and_sends_something(client, namespace, - environment, endpoint, - application): + environment, application): """`/status/watch` is Server-Sent Events, not JSON. - The only endpoint in the API that answers with a stream rather than the - result envelope, which is exactly the sort of thing a reimplementation turns - back into a plain GET without noticing. + An endpoint that answers with a stream rather than the result envelope is + exactly the sort of thing a reimplementation turns back into a plain GET + without noticing. The client refuses anything but `text/event-stream`, so + reaching an event at all is the proof; one is enough, since the status + stream follows the workload for as long as the viewer stays. """ - import requests - - url = (f"{endpoint}/api/namespaces/{namespace}/applications/{application}" - f"/status/watch?environment={environment}") - with requests.get(url, headers={"Authorization": f"Bearer {client.token}"}, - stream=True, timeout=30) as response: - assert response.status_code == 200, ( - f"the status stream answered HTTP {response.status_code}") - content_type = response.headers.get("Content-Type", "") - assert "text/event-stream" in content_type, ( - f"the status stream is served as {content_type!r} rather than " - f"text/event-stream, so a browser EventSource would refuse it") - - # One frame is enough: it proves the emitter is wired to something. - for line in response.iter_lines(decode_unicode=True): - if line: - break - # Recorded so the coverage check sees the route, which the raw request above - # bypasses. - from oops_client import record - record("GET", f"/api/namespaces/{namespace}/applications/{application}" - f"/status/watch") + stream = client.sse( + f"/api/namespaces/{namespace}/applications/{application}" + f"/status/watch?environment={environment}", timeout=30) + try: + first = next(stream, None) + finally: + stream.close() + assert first is not None, "the status stream sent nothing" @pytest.mark.cluster diff --git a/tests/integration/test_streams.py b/tests/integration/test_streams.py index 3a6caa6e..e7f23a7d 100644 --- a/tests/integration/test_streams.py +++ b/tests/integration/test_streams.py @@ -1,10 +1,15 @@ -"""WebSocket streams: pipeline logs, pod logs, and the pod terminal. +"""Streams: pipeline step status and logs over SSE, pod logs and the pod terminal. -These sockets carry the log tailing and exec multiplexing that the Kubernetes -gateways implement, and they have their own contract that is easy to get subtly -wrong on a reimplementation — in particular the ping asymmetry: log sockets -answer a text "ping" with "pong", terminal sockets must not, because their stdin -is live and a stray "pong" would be typed into the user's shell. +These carry the log tailing and exec multiplexing that the Kubernetes gateways +implement, and they have their own contract that is easy to get subtly wrong on +a reimplementation. The pipeline streams are server-sent events driven by a +browser EventSource, so every one of them must end with an `end` event — the +browser reconnects on its own after a closed response, and without `end` a +finished step would be refetched forever — and a reconnect carries the last +event id back, after which nothing may be replayed. The sockets have the ping +asymmetry: the pod log socket answers a text "ping" with "pong", the terminal +must not, because its stdin is live and a stray "pong" would be typed into the +user's shell. """ from __future__ import annotations @@ -25,9 +30,9 @@ def deployed_application(client, namespace, environment): """One application, deployed once, shared by every test in this file. - All four tests need the same thing — a running workload with a finished + Every test here needs the same thing — a running workload with a finished pipeline behind it — and a build takes about ninety seconds, so deploying per - test would spend six minutes proving the same precondition four times. None + test would spend minutes proving the same precondition over and over. None of these tests mutate the application, so sharing is safe. """ application = f"stream-{uuid.uuid4().hex[:8]}" @@ -74,45 +79,136 @@ def first_running_pod(client, namespace, application, environment): return None -def test_pipeline_log_frames_are_tagged_json(client, namespace, environment, - deployed_application): +def pipeline_events(client, namespace, application, pipeline_id, suffix, + headers=None, timeout=60): + """Every event of one pipeline stream, heartbeats dropped. + + Read to the end rather than to a count: on a finished build both streams + end by themselves, and the `end` event is the part of the contract that + matters most, so a stream that never sends it fails here on the timeout. + """ + path = (f"/api/namespaces/{namespace}/applications/{application}" + f"/pipelines/{pipeline_id}/{suffix}") + return [event for event in client.sse(path, timeout=timeout, headers=headers) + if event.event != "heartbeat"] + + +def log_lines(events): + return [line for event in events if event.event == "log" + for line in json.loads(event.data)["lines"]] + + +def test_pipeline_steps_stream_announces_steps_then_status_then_ends( + client, namespace, environment, deployed_application): application, pipeline_id = deployed_application - socket = client.websocket( - f"/api/namespaces/{namespace}/applications/{application}" - f"/pipelines/{pipeline_id}/log") - try: - frames = [] - for frame in read_until_closed(socket, timeout=30): - frames.append(frame) - if len(frames) >= 5: - break - finally: - socket.close() + events = pipeline_events(client, namespace, application, pipeline_id, + "steps/watch") - assert frames, "the pipeline log socket delivered nothing" - for frame in frames: - payload = json.loads(frame) - assert "type" in payload, f"frame without a type field: {frame[:200]}" - assert payload["type"] in ("steps", "step", "error"), ( - f"unexpected frame type {payload['type']!r}") + kinds = [event.event for event in events] + assert kinds and kinds[0] == "steps", f"stream did not open with steps: {kinds}" + assert kinds[-1] == "end", f"stream did not end with end: {kinds}" + assert "error" not in kinds, [event.data for event in events + if event.event == "error"] + steps = json.loads(events[0].data) + # A build fetches the source first, publishes the image, and finishes on + # the trailing `done` container, which is a step like any other. The names + # are the contract: the step bar shows them as they are. + assert steps[0] == "fetch" and "publish" in steps and steps[-1] == "done", steps -def test_log_socket_answers_ping_with_pong(client, namespace, environment, - deployed_application): + snapshots = [json.loads(event.data) for event in events + if event.event == "status"] + assert snapshots, "a finished build sent no status snapshot" + final = snapshots[-1] + assert final["phase"] == "Succeeded", final + by_name = {step["name"]: step for step in final["steps"]} + assert list(by_name) == steps, ( + f"status steps {list(by_name)} do not line up with steps {steps}") + for name, step in by_name.items(): + assert step["state"] == "SUCCEEDED", f"{name}: {step}" + assert step["exitCode"] == 0, f"{name}: {step}" + assert step["startedAt"] and step["finishedAt"], f"{name}: {step}" + + +def test_pipeline_step_log_replays_stamped_batches_and_ends( + client, namespace, environment, deployed_application): application, pipeline_id = deployed_application + events = pipeline_events(client, namespace, application, pipeline_id, + "log?container=fetch") + + kinds = [event.event for event in events] + assert kinds and kinds[-1] == "end", f"stream did not end with end: {kinds}" + assert "error" not in kinds, [event.data for event in events + if event.event == "error"] + batches = [event for event in events if event.event == "log"] + assert batches, "a finished fetch step replayed no log at all" + + for batch in batches: + lines = json.loads(batch.data)["lines"] + assert lines, "an empty log batch was sent" + for line in lines: + assert set(line) >= {"time", "text"}, line + assert "\n" not in line["text"], ( + f"a log line carried an embedded newline: {line['text'][:200]!r}") + # The event id is what the browser hands back on a reconnect, so it has + # to be the stamp of the last line the batch delivered. + assert batch.id == lines[-1]["time"], (batch.id, lines[-1]) + + times = [line["time"] for line in log_lines(events)] + assert times == sorted(times), "log lines arrived out of time order" + + +def test_pipeline_step_log_resumes_after_the_last_event_id( + client, namespace, environment, deployed_application): + """What the browser does on a dropped connection: reopen the stream with + Last-Event-ID set. Resumed after the final line, nothing may come back but + the end — otherwise every reconnect would replay the whole step.""" + application, pipeline_id = deployed_application + first = pipeline_events(client, namespace, application, pipeline_id, + "log?container=fetch") + last_id = [event.id for event in first if event.event == "log"][-1] + + resumed = pipeline_events(client, namespace, application, pipeline_id, + "log?container=fetch", + headers={"Last-Event-ID": last_id}) + + assert [event.event for event in resumed][-1:] == ["end"] + replayed = log_lines(resumed) + assert not replayed, ( + f"resuming after {last_id} replayed {len(replayed)} lines, the first " + f"stamped {replayed[0]['time']}") + + +def test_pipeline_step_log_of_an_unknown_step_is_an_error_then_end( + client, namespace, environment, deployed_application): + application, pipeline_id = deployed_application + events = pipeline_events(client, namespace, application, pipeline_id, + "log?container=no-such-step") + + assert [event.event for event in events] == ["error", "end"], events + assert "no-such-step" in events[0].data, events[0].data + + +def test_pod_log_socket_answers_ping_with_pong(client, namespace, environment, + deployed_application): + application, _ = deployed_application + pod = wait_until( + lambda: first_running_pod(client, namespace, application, environment), + timeout=180, interval=5, description="a running pod to appear") + socket = client.websocket( - f"/api/namespaces/{namespace}/applications/{application}" - f"/pipelines/{pipeline_id}/log") + f"/api/namespaces/{namespace}/applications/{application}/pods/{pod}/log" + f"?environment={environment}") try: socket.send("ping") - deadline_frames = [] + frames_seen = [] for frame in read_until_closed(socket, timeout=15): - deadline_frames.append(frame) + frames_seen.append(frame) if frame == "pong": break - assert "pong" in deadline_frames, ( - "the log socket did not answer a text ping with pong; the browser " - f"keepalive depends on it. Frames seen: {deadline_frames[:5]}") + assert "pong" in frames_seen, ( + "the pod log socket did not answer a text ping with pong; the browser " + f"keepalive depends on it. Frames seen: {frames_seen[:5]}") finally: socket.close() @@ -125,7 +221,8 @@ def test_pod_log_socket_streams_text_lines(client, namespace, environment, timeout=180, interval=5, description="a running pod to appear") socket = client.websocket( - f"/api/namespaces/{namespace}/applications/{application}/pods/{pod}/log") + f"/api/namespaces/{namespace}/applications/{application}/pods/{pod}/log" + f"?environment={environment}") try: frames = [frame for _, frame in zip(range(3), read_until_closed(socket, 20))] finally: @@ -133,6 +230,7 @@ def test_pod_log_socket_streams_text_lines(client, namespace, environment, # Content depends on the image, so assert the transport rather than the text: # lines arrive as text frames and are split, never glued together by a redraw. + assert frames, "the pod log socket delivered nothing" for frame in frames: assert isinstance(frame, str) assert "\n" not in frame.rstrip("\n"), ( @@ -150,7 +248,8 @@ def test_terminal_socket_does_not_answer_ping(client, namespace, environment, timeout=180, interval=5, description="a running pod to appear") socket = client.websocket( - f"/api/namespaces/{namespace}/applications/{application}/pods/{pod}/terminal") + f"/api/namespaces/{namespace}/applications/{application}/pods/{pod}/terminal" + f"?environment={environment}") try: socket.send("ping") answered = [frame for frame in read_until_closed(socket, timeout=8) diff --git a/tests/integration/test_zz_coverage.py b/tests/integration/test_zz_coverage.py index 3c0d42b8..68ad66bd 100644 --- a/tests/integration/test_zz_coverage.py +++ b/tests/integration/test_zz_coverage.py @@ -105,13 +105,18 @@ def test_every_endpoint_is_exercised_by_a_scenario(request): f"them in UNREACHABLE with the reason they cannot be driven.") -def test_coverage_report(request, record_property): - """Always-passing companion that puts the number in the report.""" +def test_coverage_report(record_testsuite_property): + """Always-passing companion that puts the number in the report. + + Recorded on the suite rather than the test case: the xunit2 schema the + JUnit file follows has no room for per-case properties, and the figure + describes the whole run anyway. + """ routes = load_routes() reached = called_keys() covered = len([r for r in routes if r["key"] in reached]) percentage = 100.0 * covered / len(routes) if routes else 0.0 - record_property("endpoints_total", len(routes)) - record_property("endpoints_covered", covered) - record_property("endpoint_coverage_percent", round(percentage, 1)) + record_testsuite_property("endpoints_total", len(routes)) + record_testsuite_property("endpoints_covered", covered) + record_testsuite_property("endpoint_coverage_percent", round(percentage, 1)) print(f"\nendpoint coverage: {covered}/{len(routes)} ({percentage:.1f}%)")