Mount testing-jiras into agent containers for e2e mock repos#565
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Docker Compose configuration and documentation to support running end-to-end tests via compose by mounting mock repositories directly, resolving issues with symlinks under Podman. Feedback highlights that the BACKPORT_MOCK_REPOS_DIR environment variable, while documented in the README, was omitted from the compose.yaml environment configuration and should be added.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces environment variables (TESTING_JIRAS_DIR, MOCK_REPOS_DIR, and BACKPORT_MOCK_REPOS_DIR) and volume mounts in compose.yaml to support containerized end-to-end testing with mock repositories, alongside updating the documentation. However, a critical issue was identified where the backport end-to-end tests will fail because the underlying framework only reads MOCK_REPOS_DIR. To resolve this, MOCK_REPOS_DIR needs to be explicitly overridden with the value of BACKPORT_MOCK_REPOS_DIR for the backport-agent-e2e-tests service in compose.yaml.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Symlinks to host paths don't work inside podman containers because bind mounts don't follow symlinks pointing outside the mounted tree. Mount the testing-jiras directory directly via TESTING_JIRAS_DIR and pass MOCK_REPOS_DIR to agent containers so fixture configs are found. Pass BACKPORT_MOCK_REPOS_DIR to the backport test service separately, since the backport test reads that variable instead of MOCK_REPOS_DIR. Guard against empty env var values in both test suites so they fall back to DEFAULT_FIXTURES_DIR instead of loading from an empty path. Assisted-by: Claude
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configuration updates to support mounting a testing repository (TESTING_JIRAS_DIR) via Docker Compose, updating environment variable fallbacks in the E2E tests to correctly handle empty strings, and documenting these setup steps. The reviewer's feedback recommends refactoring the redundant environment variable lookups in test_backport.py into a single module-level FIXTURES_DIR constant, as well as adding defensive checks in both test_backport.py and test_triage.py to ensure the fixtures directory exists before running tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
|
|
||
| test_cases = _load_test_cases(os.getenv("BACKPORT_MOCK_REPOS_DIR", str(DEFAULT_FIXTURES_DIR))) | ||
| test_cases = _load_test_cases(os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR)) |
There was a problem hiding this comment.
Instead of repeating the environment variable lookup and fallback logic across multiple functions, define a single module-level FIXTURES_DIR constant. Additionally, add a defensive check to ensure the directory exists, preventing silent test collection failures or empty test runs if the environment variable is misconfigured.
FIXTURES_DIR = Path(os.getenv("BACKPORT_MOCK_REPOS_DIR") or DEFAULT_FIXTURES_DIR)
if not FIXTURES_DIR.is_dir():
raise FileNotFoundError(f"Fixtures directory not found: {FIXTURES_DIR}")
test_cases = _load_test_cases(FIXTURES_DIR)| Control to the test session after repos are prepared. | ||
| """ | ||
| fixtures_dir = os.getenv("BACKPORT_MOCK_REPOS_DIR", str(DEFAULT_FIXTURES_DIR)) | ||
| fixtures_dir = os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR) |
There was a problem hiding this comment.
Use the module-level FIXTURES_DIR constant to avoid redundant environment variable lookups and ensure consistency.
| fixtures_dir = os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR) | |
| fixtures_dir = FIXTURES_DIR |
| if not ref_patch_rel: | ||
| return None | ||
| fixtures_dir = Path(os.getenv("BACKPORT_MOCK_REPOS_DIR", str(DEFAULT_FIXTURES_DIR))) | ||
| fixtures_dir = Path(os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR)) |
There was a problem hiding this comment.
Use the module-level FIXTURES_DIR constant to avoid redundant environment variable lookups and ensure consistency.
| fixtures_dir = Path(os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR)) | |
| fixtures_dir = FIXTURES_DIR |
| Control to the test session after repos are prepared. | ||
| """ | ||
| fixtures_dir = os.getenv("MOCK_REPOS_DIR", str(DEFAULT_FIXTURES_DIR)) | ||
| fixtures_dir = os.getenv("MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR) |
There was a problem hiding this comment.
Add a defensive check to ensure the fixtures directory exists. This prevents confusing downstream errors (like git clone/fetch failures) if MOCK_REPOS_DIR is misconfigured.
| fixtures_dir = os.getenv("MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR) | |
| fixtures_dir = Path(os.getenv("MOCK_REPOS_DIR") or DEFAULT_FIXTURES_DIR) | |
| if not fixtures_dir.is_dir(): | |
| raise FileNotFoundError(f"Fixtures directory not found: {fixtures_dir}") |
1 participant
Symlinks to host paths don't work inside podman containers because bind mounts don't follow symlinks pointing outside the mounted tree. Mount the testing-jiras directory directly via TESTING_JIRAS_DIR and pass MOCK_REPOS_DIR to agent containers so fixture configs are found. Also update both READMEs to document the compose-based setup alongside the host symlink approach.
Assisted-by: Claude