Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ x-beeai-env: &beeai-env
REDIS_URL: redis://valkey:6379/0
COLLECTOR_ENDPOINT: http://otel-collector:4318/v1/traces
GIT_REPO_BASEPATH: /git-repos
MOCK_REPOS_DIR: ${MOCK_REPOS_DIR:-}
Comment thread
grulja marked this conversation as resolved.
MAX_RETRIES: 3
DRY_RUN: ${DRY_RUN:-false}
JIRA_DRY_RUN: ${JIRA_DRY_RUN:-false}
Expand All @@ -28,6 +29,7 @@ x-beeai-agent: &beeai-agent
- ./ymir/tools:/home/beeai/ymir/tools:ro,z
- ./ymir/common:/home/beeai/ymir/common:ro,z
- git-repos:/git-repos:rw,U
- ${TESTING_JIRAS_DIR:-./ymir/agents/tests/e2e/mock_repos}:/testing-jiras:ro,z
- .secrets/rhel-config.json:/home/beeai/rhel-config.json:ro,z,U
- .secrets/jotnar-vertex-dev.json:/home/beeai/jotnar-vertex-dev.json:ro,z,U
restart: unless-stopped
Expand Down Expand Up @@ -212,6 +214,7 @@ services:
<<: *beeai-agent-c10s
environment:
<<: *beeai-env
BACKPORT_MOCK_REPOS_DIR: ${BACKPORT_MOCK_REPOS_DIR:-}
command: ["pytest", "ymir/agents/tests/e2e/backport_agent/test_backport.py", "-o", "asyncio_default_test_loop_scope=session"]
profiles: ["e2e-test"]

Expand Down
6 changes: 3 additions & 3 deletions ymir/agents/tests/e2e/backport_agent/test_backport.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def _load_test_cases(fixtures_dir: str | Path) -> list[BackportAgentTestCase]:
return cases


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))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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)



@pytest.fixture(scope="session", autouse=True)
Expand All @@ -116,7 +116,7 @@ def mock_centos_stream_repos():
Yields:
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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use the module-level FIXTURES_DIR constant to avoid redundant environment variable lookups and ensure consistency.

Suggested change
fixtures_dir = os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR)
fixtures_dir = FIXTURES_DIR

configs = load_all_fixture_configs(fixtures_dir)

if SHARED_BARE_REPOS_DIR.exists():
Expand Down Expand Up @@ -151,7 +151,7 @@ def _load_reference_patch(test_case: "BackportAgentTestCase") -> str | None:
ref_patch_rel = test_case.expected.get("reference_patch")
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))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use the module-level FIXTURES_DIR constant to avoid redundant environment variable lookups and ensure consistency.

Suggested change
fixtures_dir = Path(os.getenv("BACKPORT_MOCK_REPOS_DIR") or str(DEFAULT_FIXTURES_DIR))
fixtures_dir = FIXTURES_DIR

ref_patch_path = fixtures_dir / ref_patch_rel
if ref_patch_path.is_file():
return ref_patch_path.read_text()
Expand Down
23 changes: 18 additions & 5 deletions ymir/agents/tests/e2e/mock_repos/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,25 @@ This directory contains mock repo fixture files organized by agent type.
The fixtures come from `git@gitlab.cee.redhat.com:jotnar-project/testing-jiras.git`
if you have access.

Clone the repository and symlink its `mock_data/` subdirectories here:
Clone the repository and either:

```bash
ln -s /path/to/testing-jiras/mock_data/triage ymir/agents/tests/e2e/mock_repos/triage
ln -s /path/to/testing-jiras/mock_data/backport ymir/agents/tests/e2e/mock_repos/backport
```
- **Run via compose (recommended):** set `TESTING_JIRAS_DIR` in your local `.env` to point
at the cloned repository root so compose mounts it into the container:
```
TESTING_JIRAS_DIR=/path/to/testing-jiras
MOCK_REPOS_DIR=/testing-jiras/mock_data/triage
BACKPORT_MOCK_REPOS_DIR=/testing-jiras/mock_data/backport
```

- **Run directly on host:** symlink the `mock_data/` subdirectories here:
```bash
ln -s /path/to/testing-jiras/mock_data/triage ymir/agents/tests/e2e/mock_repos/triage
ln -s /path/to/testing-jiras/mock_data/backport ymir/agents/tests/e2e/mock_repos/backport
```

> **Note:** Symlinks only work when running tests on the host. Podman bind
> mounts do not follow symlinks pointing outside the mounted tree, so
> containerized e2e tests require the compose-based approach.

## Expected layout

Expand Down
2 changes: 1 addition & 1 deletion ymir/agents/tests/e2e/test_triage.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def mock_centos_stream_repos(tmp_path_factory):
Yields:
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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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}")

configs = load_all_fixture_configs(fixtures_dir)

if git_repo_basepath := os.getenv("GIT_REPO_BASEPATH"):
Expand Down
5 changes: 3 additions & 2 deletions ymir/tools/privileged/tests/data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ This directory is meant to contain Jira mock files from

Clone the repository and either:

- **Run via compose (recommended):** set `JIRA_MOCK_FILES_HOST` in your local `.env` to point
at the cloned `jiras/` directory so compose mounts it directly into the container:
- **Run via compose (recommended):** set `TESTING_JIRAS_DIR` and `JIRA_MOCK_FILES_HOST` in
your local `.env` to point at the cloned repository:
```
TESTING_JIRAS_DIR=/path/to/testing-jiras
JIRA_MOCK_FILES_HOST=/path/to/testing-jiras/jiras
```

Expand Down
Loading