chore: make selenium container management more reliable#2603
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make the Selenium-based UI test container lifecycle more reliable by introducing explicit container health checks and centralized container naming, while also moving Selenium pytest CLI options from pyproject.toml into the ui-selenium Taskfile task.
Changes:
- Add Podman healthcheck-based startup gating for the Selenium container and reuse a shared
CONTAINER_NAMEconstant. - Adjust Selenium test execution defaults by simplifying
[tool.pytest].addoptsand moving coverage/junit args intotask ui-selenium. - Minor test/logging tweaks (logger names and disabling a flaky marker via commenting).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/selenium/utils/settings_utils.py | Switch logger naming to use __package__. |
| test/selenium/ui/test_lightspeed.py | Comment out the pytest.mark.flaky decorator for one test. |
| test/selenium/fixtures/ui_fixtures.py | Add container healthcheck gating and restart logic before creating Remote WebDriver. |
| test/selenium/const.py | Introduce CONTAINER_NAME = "selenium-vscode". |
| test/selenium/conftest.py | Use shared container name; adjust logging format; change teardown to stop container and capture stats output. |
| pyproject.toml | Simplify pytest addopts to reduce IDE/console friction; remove UI-selenium-specific options. |
| Taskfile.yml | Make task ui-selenium explicitly pass coverage/junit args to pytest instead of relying on pyproject.toml. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{.CLI_ARGS}} | ||
| # --reruns-delay=120 | ||
| # --reruns=3 | ||
| - defer: podman-compose down |
There was a problem hiding this comment.
This task still uses defer: podman-compose down, but the new test teardown comments note that podman-compose down can fail intermittently. To avoid CI flakes from teardown, consider making this teardown non-fatal (e.g., podman-compose down || true) or switching it to the same podman stop ${CONTAINER_NAME} approach used in the pytest session teardown.
| - defer: podman-compose down | |
| - defer: podman-compose down || true |
| count = 0 | ||
| while True: | ||
| if is_container_healthy(): | ||
| break | ||
| count += 1 | ||
| time.sleep(1) | ||
| log.info( | ||
| "Waiting for container %s to be healthy: %s", CONTAINER_NAME, count | ||
| ) |
There was a problem hiding this comment.
The wait loop for container health has no timeout (while True) and will hang the entire test session indefinitely if the container never becomes healthy (e.g., image pull issues, healthcheck misconfig, podman errors). Add a max wait/retry limit and fail fast with a clear error that includes relevant diagnostics (e.g., last healthcheck stdout/stderr, podman logs, podman inspect health status).
| result = subprocess.run( | ||
| f"podman stop {CONTAINER_NAME} 2>/dev/null || true", | ||
| # Apparently compose down can fail with various errors but stopping container works | ||
| # "podman-compose down", | ||
| capture_output=True, | ||
| check=False, | ||
| shell=True, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
podman stop ... || true forces a zero exit code even when podman stop fails, so if result.returncode != 0: will never trigger and failures to stop the container will be silently ignored. Remove the || true (and rely on check=False) or explicitly handle the "no such container" case so real stop failures can be detected/logged.
1 participant
No description provided.