ci(test): Verify per-session video layout for Dynamic Grid (Docker + K8s)

[VietND96]

Description

Adds test coverage asserting that Dynamic Grid recordings are stored one per session at <assets>/<sessionId>/<file>.mp4 — never flat in the assets root and never double-nested — covering SE_VIDEO_SESSION_SUBFOLDER behaviour for both inline recording and the external video sidecar.

Verifies the Grid-side change SeleniumHQ/selenium@47edefb ("Store Dynamic Grid videos in a per-session subfolder via SE_VIDEO_SESSION_SUBFOLDER").

Changes

• Kubernetes Dynamic Grid test (tests/k8s/make/dynamic_grid_test.sh): collect videos cluster-agnostically — kubectl exec/cp from the assets pod on any cluster, hostPath fast path on minikube — preserving the per-session layout, then assert every recording sits in a per-session subfolder before the ffmpeg integrity check.
• Docker Dynamic Grid test (Makefile): new CHECK_VIDEO_SESSION_LAYOUT gate that fails on a flat (depth 1, inline regression) or double-nested (depth ≥3, sidecar regression) recording, and requires ≥1 per-session recording. Enabled on the inline (test_standalone_docker) and external-sidecar (test_standalone_docker_video_sidecar, test_node_docker_video_sidecar) scenarios.
• Retain local recordings for the assertion via SE_UPLOAD_RETAIN_LOCAL_FILE (default false, no behaviour change elsewhere) passed through the node/standalone-docker compose files, so the per-session recordings persist alongside the upload for verification.

Both location and ffmpeg output integrity are checked in every scenario.

Verification

Ran locally (arm64, image tag 4.46.0-20260804):

Scenario	Result
Docker inline (video-image="false")	✅ 6/6 recordings per-session, none flat; 12 files ffmpeg-valid
Docker external sidecar	✅ 6/6 recordings per-session, none double-nested
K8s Standalone Dynamic Grid	✅ per-session subfolders, ffmpeg-valid

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

CI tests: assert Dynamic Grid videos are stored per-session (Docker + K8s)

🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Add CI assertions that recordings live under "//*.mp4" (no flat or double-nested paths).
• Preserve per-session folder structure when collecting K8s Dynamic Grid assets across cluster
 types.
• Pass through "SE_UPLOAD_RETAIN_LOCAL_FILE" so uploaded recordings remain available for
 verification.

Diagram

graph TD
  A["Makefile (test_*)"] --> B["Docker Compose test run"] --> C[("Video assets (./tests/videos)")] --> D["Session layout check"] --> E["ffmpeg integrity check"]
  F["K8s dynamic_grid_test.sh"] --> G["kubectl exec/cp (assets pod)"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Extract a shared "video layout" validation script

• ➕ Avoids duplicating layout rules across Makefile and K8s script
• ➕ Enables reuse from other scenarios (e.g., chart tests, future CI jobs)
• ➕ Simplifies future changes to ignore paths/exclusions
• ➖ Introduces an extra helper file and wiring to call it from both paths
• ➖ Slightly more indirection when debugging CI failures

2. Validate layout inside the container/pod only (no host-side copies)

• ➕ Reduces host filesystem manipulation and path translation issues
• ➕ Can be faster by avoiding multiple kubectl cp operations
• ➖ Harder to archive/inspect artifacts when tests fail
• ➖ Still needs special casing for minikube hostPath vs non-minikube PVC mounts

Recommendation: Current approach is solid for CI because it validates the same artifacts later fed into ffmpeg checks and preserves per-session structure during collection. If this pattern expands to more environments, extracting a shared validation helper would reduce duplication and keep the rules consistent.

Files changed (4) +69 / -3

Tests (2) +67 / -3

MakefileGate Docker Dynamic Grid tests on per-session video layout +15/-1

Gate Docker Dynamic Grid tests on per-session video layout

• Adds SE_UPLOAD_RETAIN_LOCAL_FILE to the Docker test environment and introduces CHECK_VIDEO_SESSION_LAYOUT. When enabled, the test run fails if MP4s are flat in ./tests/videos, double-nested below <sessionId>/, or missing entirely, before running the existing ffmpeg integrity check.

Makefile

dynamic_grid_test.shCollect K8s Dynamic Grid videos preserving per-session folders and assert layout +52/-2

Collect K8s Dynamic Grid videos preserving per-session folders and assert layout

• Updates K8s Dynamic Grid CI to collect MP4s in a cluster-agnostic way: hostPath copy on minikube and kubectl exec/cp from the assets-mounting pod otherwise. Preserves the per-session directory layout during collection and asserts videos are not stored flat in the assets root before running ffmpeg integrity checks.

tests/k8s/make/dynamic_grid_test.sh

Other (2) +2 / -0

docker-compose-v3-test-node-docker.yamlPropagate SE_UPLOAD_RETAIN_LOCAL_FILE into node-docker test container +1/-0

Propagate SE_UPLOAD_RETAIN_LOCAL_FILE into node-docker test container

• Adds SE_UPLOAD_RETAIN_LOCAL_FILE (default false) so videos can be retained locally even when upload is enabled, enabling post-run layout verification.

tests/docker-compose-v3-test-node-docker.yaml

docker-compose-v3-test-standalone-docker.yamlPropagate SE_UPLOAD_RETAIN_LOCAL_FILE into standalone-docker test container +1/-0

Propagate SE_UPLOAD_RETAIN_LOCAL_FILE into standalone-docker test container

• Adds SE_UPLOAD_RETAIN_LOCAL_FILE (default false) to keep local recordings alongside uploads for layout assertions in standalone Docker scenarios.

tests/docker-compose-v3-test-standalone-docker.yaml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Stale videos fail layout check 🐞 Bug ☼ Reliability

Description

tests/k8s/make/dynamic_grid_test.sh now validates that no .mp4 exists directly under the assets
root, but it never clears the underlying hostPath/PV directory before scanning it. Because the PV’s
backing directory is retained, stale recordings from earlier runs can trigger false failures (or
make the “videos exist” check pass even if this run produced none).

Code

tests/k8s/make/dynamic_grid_test.sh[R262-265]

+  base="${ASSETS_HOST_PATH%/}"
+  video_files=$(find "${base}" -type f -name "*.mp4")
+  # Copy while preserving the per-session subfolder layout so same-named recordings from
+  # different sessions do not collide.

Evidence

The script creates/permissions the assets directory but does not remove existing files, then later
uses find to enumerate *all* *.mp4 under the persistent assets base and fails if any are located
directly under the base. The PV backing that directory is a hostPath with
persistentVolumeReclaimPolicy: Retain, so stale recordings can persist between runs and be picked
up by this new validation.

tests/k8s/make/dynamic_grid_test.sh[149-152]
tests/k8s/make/dynamic_grid_test.sh[256-309]
kubernetes/DynamicGrid/BaseConfig/pvc.yaml[12-16]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The K8s Dynamic Grid test collects and validates all `*.mp4` under the assets base path, but the test does not clean the backing assets directory before the run. Since the PV is hostPath-backed and configured to retain data, stale `*.mp4` files from previous runs can be included in `video_files` and cause the new “flat video” assertion to fail (or satisfy the “non-empty” check without producing new recordings).

## Issue Context
- The script prepares `${ASSETS_HOST_PATH}` with `mkdir/chmod` only (no cleanup), then scans all `*.mp4` and fails if any are in the assets root.
- The PV is `hostPath: /tmp/selenium/assets` with `persistentVolumeReclaimPolicy: Retain`, so artifacts can persist across runs.

## Fix Focus Areas
- tests/k8s/make/dynamic_grid_test.sh[149-152]
- tests/k8s/make/dynamic_grid_test.sh[256-309]
- kubernetes/DynamicGrid/BaseConfig/pvc.yaml[12-16]

## Suggested fix
Add an explicit cleanup step for old `*.mp4` files in the assets directory before running the browsers/tests (and/or before collecting videos). For non-minikube clusters, do the cleanup via `kubectl exec` against the pod mounting `/opt/selenium/assets` once the deployment is ready.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-code-review]

1. Stale videos fail layout check 🐞 Bug ☼ Reliability

tests/k8s/make/dynamic_grid_test.sh now validates that no .mp4 exists directly under the assets
root, but it never clears the underlying hostPath/PV directory before scanning it. Because the PV’s
backing directory is retained, stale recordings from earlier runs can trigger false failures (or
make the “videos exist” check pass even if this run produced none).

Agent Prompt

## Issue description
The K8s Dynamic Grid test collects and validates all `*.mp4` under the assets base path, but the test does not clean the backing assets directory before the run. Since the PV is hostPath-backed and configured to retain data, stale `*.mp4` files from previous runs can be included in `video_files` and cause the new “flat video” assertion to fail (or satisfy the “non-empty” check without producing new recordings).

## Issue Context
- The script prepares `${ASSETS_HOST_PATH}` with `mkdir/chmod` only (no cleanup), then scans all `*.mp4` and fails if any are in the assets root.
- The PV is `hostPath: /tmp/selenium/assets` with `persistentVolumeReclaimPolicy: Retain`, so artifacts can persist across runs.

## Fix Focus Areas
- tests/k8s/make/dynamic_grid_test.sh[149-152]
- tests/k8s/make/dynamic_grid_test.sh[256-309]
- kubernetes/DynamicGrid/BaseConfig/pvc.yaml[12-16]

## Suggested fix
Add an explicit cleanup step for old `*.mp4` files in the assets directory before running the browsers/tests (and/or before collecting videos). For non-minikube clusters, do the cleanup via `kubectl exec` against the pod mounting `/opt/selenium/assets` once the deployment is ready.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools