Skip to content

[DSPX-3302] (4/5) xtest conftest: --scenario and --instance flags #453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dmihalcik-virtru wants to merge 1 commit into
base: DSPX-3302-03-multi-instance
Choose a base branch
from
Draft

[DSPX-3302] (4/5) xtest conftest: --scenario and --instance flags #453

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 61 additions & 0 deletions xtest/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ def sdk_spec_type(v: str) -> str:

def pytest_addoption(parser: pytest.Parser):
"""Add custom CLI options for pytest."""
parser.addoption(
"--scenario",
help="path to scenarios.yaml; --sdks-encrypt/--sdks-decrypt/--containers default from it",
type=Path,
)
parser.addoption(
"--instance",
help="otdf-local instance name; sets OTDF_LOCAL_INSTANCE_NAME for child tooling",
type=str,
)
parser.addoption(
"--audit-log-dir",
help="directory to write audit logs on test failure (default: tmp/audit-logs)",
Expand Down Expand Up @@ -130,6 +140,57 @@ def pytest_addoption(parser: pytest.Parser):
)


def pytest_configure(config: pytest.Config) -> None:
"""Apply --scenario defaults and --instance env-var threading.

When `--scenario PATH` is given, missing `--sdks-encrypt`, `--sdks-decrypt`,
and `--containers` options are populated from the scenario file. Options
explicitly passed on the CLI always win. `--instance NAME` is propagated
via `OTDF_LOCAL_INSTANCE_NAME` so any child `otdf-local` invocation sees
the same instance.
"""
import os
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

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

medium

The os module is already imported at the top of the file (line 18). This local import is redundant and can be removed to improve code clarity and adhere to Python's best practices (PEP 8), which recommend placing all imports at the top of the module.


instance = config.getoption("--instance")
if instance:
os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance
Comment on lines +152 to +156
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

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

medium

The import os statement is redundant as it is already imported at the top of the file (line 18). Additionally, the check if instance: should be replaced with if instance is not None: to correctly handle cases where an empty string is explicitly passed via the CLI, ensuring it is respected and not ignored in favor of scenario defaults.

Suggested change
import os
instance = config.getoption("--instance")
if instance:
os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance
instance = config.getoption("--instance")
if instance is not None:
os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance
References
  1. Redundant imports should be removed to maintain code cleanliness and avoid confusion.
  2. When implementing configuration overrides, ensure that explicit user input (even if empty) is prioritized over default values.


scenario_path = config.getoption("--scenario")
if not scenario_path:
return
try:
from otdf_sdk_mgr.schema import (
installed_json_for,
load_scenario,
scenario_to_pytest_sdks,
)
except ImportError:
# otdf-sdk-mgr may not be installed in a minimal pytest env.
return
scenario = load_scenario(scenario_path)
# `sdk@<version>` tokens come from the install record so they match the
# dist directories #446's parser walks under `xtest/sdk/<lang>/dist/`.
# If the user passed --sdks-encrypt / --sdks-decrypt explicitly, their
# tokens win and we skip the resolution step entirely.
need_resolve = (
(not config.getoption("--sdks-encrypt") and scenario.sdks.encrypt)
or (not config.getoption("--sdks-decrypt") and scenario.sdks.decrypt)
)
if need_resolve:
try:
tokens = scenario_to_pytest_sdks(scenario, installed_json_for(scenario_path))
except FileNotFoundError as e:
raise pytest.UsageError(str(e)) from e
if not config.getoption("--sdks-encrypt") and tokens["encrypt"]:
config.option.sdks_encrypt = " ".join(tokens["encrypt"])
if not config.getoption("--sdks-decrypt") and tokens["decrypt"]:
config.option.sdks_decrypt = " ".join(tokens["decrypt"])
if not config.getoption("--containers") and scenario.suite.containers:
config.option.containers = scenario.suite.containers
if not instance and scenario.instance.metadata.name:
os.environ["OTDF_LOCAL_INSTANCE_NAME"] = scenario.instance.metadata.name
Comment on lines +152 to +191
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

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

medium

The current implementation uses falsiness checks (if not ...) to determine if CLI options were provided. This prevents users from explicitly overriding scenario defaults with empty values (e.g., --sdks-encrypt "" to run no encryption SDKs), as an empty string is falsy and will be replaced by the scenario's default. Using is None checks ensures that only unprovided options are populated from the scenario file, adhering to the principle that CLI-passed options always win.

Additionally, the local import os is redundant as it is already imported at the top of the file (line 18).

    instance = config.getoption("--instance")
    if instance is not None:
        os.environ["OTDF_LOCAL_INSTANCE_NAME"] = instance

    scenario_path = config.getoption("--scenario")
    if not scenario_path:
        return
    try:
        from otdf_sdk_mgr.schema import load_scenario
    except ImportError:
        # otdf-sdk-mgr may not be installed in a minimal pytest env.
        return
    scenario = load_scenario(scenario_path)
    if config.getoption("--sdks-encrypt") is None and scenario.sdks.encrypt:
        config.option.sdks_encrypt = " ".join(scenario.sdks.encrypt.keys())
    if config.getoption("--sdks-decrypt") is None and scenario.sdks.decrypt:
        config.option.sdks_decrypt = " ".join(scenario.sdks.decrypt.keys())
    if config.getoption("--containers") is None and scenario.suite.containers:
        config.option.containers = scenario.suite.containers
    if instance is None and scenario.instance.metadata.name:
        os.environ["OTDF_LOCAL_INSTANCE_NAME"] = scenario.instance.metadata.name



def pytest_generate_tests(metafunc: pytest.Metafunc):
"""Dynamically parametrize test functions based on CLI options.

Expand Down
Loading