Extract test scenarios into scenarios.yaml

[herczyn]

Summary

Move the hardcoded TEST_CASES list from run_tests.py into a separate scenarios.yaml data file, and grow the schema with a steps: list per test case (in preparation for ACTS-style multi-step tests where one test needs setup → action → assertion across multiple traversals).

Motivation

• Tests as data, not code. Adding or modifying a scenario no longer requires editing the runner. Consuming SDK repositories can drop in their own scenarios file via the new --scenarios CLI flag, which is the per-SDK PR-vs-nightly model the README has documented but not previously enabled in code.
• Schema growth path. The new steps: list makes each test case a sequence of traversals run sequentially against the same cluster. All 21 existing scenarios become single-step tests; nothing about their behaviour changes. This is groundwork for being able to express tests like "create a task, wait for it to enter state X, then run the actual assertion" — useful both for ITK's own future tests and as a stepping stone toward acting as an ACTS conformance runner (cf. feat: Add new spec for A2A protocol conformance tests A2A#1882).
• Backwards compatible. The legacy flat form (sdks / behavior / etc. at the top level of a test entry, no steps: block) is still accepted and auto-normalised into a single step, so existing /run HTTP callers and any pre-existing scenarios.json files keep working.

Changes

File	What
scenarios.yaml (new)	All 21 previously-hardcoded scenarios, in YAML, with the two known-failing entries (failing-go-v03-http-json, failing-go-v10-grpc) marked expected: fail so the runner keeps exercising them and alerts when they start passing again.
itk_service.py	TestCase split into TestCase (named, multi-step, optional expected: pass|fail) + TestStep (the per-traversal shape). New _run_test_case() helper shared by run_tests.py and the /run endpoint. Backwards-compat shim auto-normalises the legacy flat form.
run_tests.py	Reduced from 274 lines to a thin CLI that loads YAML, validates via TestCase, and delegates to _run_test_case. New --scenarios <path> flag (defaults to scenarios.yaml next to the script).
pyproject.toml	Add pyyaml>=6.0 dependency.
README.md	Document the YAML format (full field reference + legacy flat-form note) and the --scenarios flag.

Validation

Sanity-checked locally:

• All 21 scenarios round-trip through the pydantic schema and preserve the original sdks / behavior / protocols / edges / streaming / build_subtests values in the right order.
• Both known-failing cases are flagged as expected: fail.
• Legacy flat form (no steps: block) auto-normalises to a single-step test.
• Multi-step form works.
• Empty / invalid cases are correctly rejected at parse time.
• uv run run_tests.py --help works.
• Code passes ruff with the project config.

I did not run the actual integration tests (uv run run_tests.py) end-to-end as that requires the full multi-language container environment; this PR is intentionally pure refactor and the runtime call into execute_itk_test is unchanged.

Out of scope (future PRs)

• Wiring the per-SDK scenarios.yaml model into the consuming SDK repos (a2a-python, a2a-go, ...) — none of them currently have an itk/ directory or any ITK-specific files on disk yet, contrary to what the README implies. That's a separate coordination effort.
• Multi-step test cases that exercise the new shape. Adding the first real one (e.g. an ACTS-style tck-cancel → poll-for-state → cancel_task test) belongs in a follow-up.
• Becoming an ACTS conformance runner (cf. #1882 in a2aproject/A2A). This PR is a prerequisite step toward that, not the thing itself.

[gemini-code-assist]

Code Review

This pull request refactors the ITK integration test orchestrator to dynamically load test scenarios from a YAML file (scenarios.yaml) instead of using hardcoded definitions. It introduces a multi-step test execution schema (TestStep) with backwards compatibility for the legacy flat form, supported by Pydantic models in itk_service.py. Feedback on the changes highlights a logic bug in how expected-failing test cases invert individual step results, a potential FileNotFoundError caused by using forward slashes in step labels, and several robustness issues in the YAML loading and validation logic in run_tests.py.

[gemini-code-assist]

Inverting the passed flag of every individual step/subtest in an expected-failing test case (expected: fail) introduces a logic bug for multi-step or subtest scenarios.

If a test case has multiple steps (e.g., Step 1 passes, Step 2 fails), the overall test case has successfully failed (meeting the expectation). However, inverting each step individually results in Step 1 becoming passed: False and Step 2 becoming passed: True. Because the runner checks if all entries are successful, the presence of passed: False on Step 1 will cause the entire test run to fail.

To fix this, we should determine if the test case actually failed (i.e., at least one step/subtest failed). If it did, we report all steps as passed: True (expectation met). If all steps passed, we report all steps as passed: False (expectation not met).

Suggested change

	if case.expected == 'fail':
	for entry in merged.values():
	entry['passed'] = not entry['passed']
	entry['expected'] = 'fail'
	if case.expected == 'fail':
	actually_failed = any(not entry['passed'] for entry in merged.values())
	for entry in merged.values():
	entry['passed'] = actually_failed
	entry['expected'] = 'fail'

[herczyn]

Good catch — accepted in 682c27b. The latent bug only matters for multi-step cases or single steps with build_subtests: true, neither of which are exercised by the 21 scenarios in this PR, so behaviour is unchanged for them. But it would have produced incoherent results for any future test in either of those shapes, so worth fixing now.

Verified with mocked execute_itk_test across 7 scenarios:

• single-step expected: fail that actually fails → passed: true ✓
• single-step expected: fail that actually passes → passed: false ✓
• multi-step expected: fail where one of two steps fails → both entries get passed: true ✓
• multi-step expected: fail where all steps pass → all entries get passed: false ✓
• build_subtests + expected: fail with mixed pass/fail subtests → all entries get passed: true ✓

[gemini-code-assist]

Using a forward slash / in the step label (e.g., f'{case.name}/step-{idx}') causes a FileNotFoundError when running with ITK_LOG_LEVEL=DEBUG.

In testlib.py, start_notification_server attempts to write logs to os.path.join(cwd, 'logs', f'{test_name}_notifications.log'). If test_name contains a slash, Python interprets it as a subdirectory path (e.g., logs/my-scenario/step-1_notifications.log). Since logs/my-scenario/ is not created, open() fails.

Replacing the slash with a hyphen or underscore avoids this issue while keeping the step label clear and distinguishable.

Suggested change

	label = (
	f'{case.name}/step-{idx}' if multi_step else case.name
	)
	label = (
	f'{case.name}-step-{idx}' if multi_step else case.name
	)

[herczyn]

Accepted in 682c27b. Switched to -step- instead of /step-. Doesn't collide with the existing -sub- suffix that execute_itk_test uses for build_subtests, so process_results.py's name.split('-sub-')[0] parent-lookup still works unchanged.

[gemini-code-assist]

The load_scenarios function is prone to crashes and unhandled exceptions:

1. yaml.safe_load(f) is called outside the try...except block, meaning any YAML syntax/parsing errors will crash the runner with a traceback instead of a clean error message.
2. If the YAML file is not a dictionary (e.g., a list or a string), data.get('tests') will raise an AttributeError.
3. There is no validation to ensure test case names are unique, which can lead to duplicate test cases silently overwriting each other in the results map.

Wrapping the entire loading and validation logic in the try...except block, validating the data type, and checking for duplicate test names makes the runner much more robust.

def load_scenarios(path: pathlib.Path) -> list[TestCase]:
    """Load and validate a scenarios file.

    The file is expected to have a top-level ``tests:`` list, each entry
    of which conforms to :class:`TestCase` (either the multi-step or
    legacy flat form).
    """
    if not path.exists():
        logger.error('Scenarios file not found: %s', path)
        sys.exit(1)
    try:
        with path.open() as f:
            data = yaml.safe_load(f)
        if data is None:
            data = {}
        elif not isinstance(data, dict):
            logger.error("Scenarios file %s must be a YAML dictionary.", path)
            sys.exit(1)
        raw_tests = data.get('tests')
        if not isinstance(raw_tests, list) or not raw_tests:
            logger.error(
                "Scenarios file %s must contain a non-empty 'tests' list.", path
            )
            sys.exit(1)
        
        tests = [TestCase.model_validate(entry) for entry in raw_tests]
        names = [t.name for t in tests]
        duplicates = {name for name in names if names.count(name) > 1}
        if duplicates:
            logger.error("Duplicate test case names found in %s: %s", path, ", ".join(duplicates))
            sys.exit(1)
        return tests
    except Exception:
        logger.exception('Failed to parse scenarios from %s', path)
        sys.exit(1)

[herczyn]

All three sub-issues accepted in 682c27b:

1. yaml.safe_load is now inside the try block — YAML syntax errors produce a clean logged error and sys.exit(1) instead of an unhandled yaml.YAMLError traceback.
2. Added an explicit isinstance(data, dict) check before .get('tests'). List-rooted or scalar-rooted YAML now reports the actual type in the error message instead of raising AttributeError.
3. Added a duplicate-name check using collections.Counter (O(n) rather than O(n²) — the suggestion's names.count(name) inside a comprehension is O(n²)).

Verified with 9 negative scenarios — missing file, list-rooted YAML, scalar-rooted YAML, empty file, malformed YAML, missing tests key, empty tests list, pydantic validation failure, and duplicate names — all produce a single logged error line and exit 1 with no unhandled exceptions. The real scenarios.yaml (21 cases) still loads.

One small structural difference from the suggested patch: I kept the existing path.exists() check outside the try block (it has its own targeted error message) and added an inner except SystemExit: raise so the explicit sys.exit(1) calls inside the try aren't accidentally swallowed by the broad except Exception.