test#23
Conversation
| raise ValueError(f"venv dir already exists (no overwrite): {venv_dir}") | ||
| python_bin = _ci_runtime_python_executable() | ||
| run_subprocess([python_bin, "-m", "venv", str(venv_dir)]) | ||
| # Create the CI venv without ensurepip. GitHub-hosted Python 3.10 can fail inside |
There was a problem hiding this comment.
Creating the venv with --system-site-packages --without-pip makes runtime setup depend on the host environment, while _prepare_ci_runtime_python_env still shells out to python -m pip install. Please prove that pip is always available in this mode, or keep the venv self contained and fix the ensurepip failure another way.
| def _run_subprocess(argv: List[str], *, cwd: str) -> None: | ||
| print("RUN:", " ".join(_shell_quote(a) for a in argv), flush=True) | ||
| proc = subprocess.run(argv, cwd=cwd) | ||
| proc = subprocess.run(argv, cwd=cwd, capture_output=True, text=True) |
There was a problem hiding this comment.
Capturing all subprocess output here turns long-running CI/testbed commands into buffered jobs and hides live logs. That regresses observability for every wrapper using _run_subprocess; if you only need better failure detail, tee output or capture only on the error path.
| ) | ||
| return | ||
| command_text = command.get("command") | ||
| if not isinstance(command_text, str) or "_cargo_kv_unit.py --case-config __RUN_DIR__/configs/ci_scene_config.yaml" not in command_text: |
There was a problem hiding this comment.
This only checks the generated command string, not the wrapper contract. The test will still pass if _cargo_kv_unit.py ignores scene_config and reads env/defaults instead, which is the behavior this PR should be pinning down. Consider asserting the wrapper behavior directly.
| }, | ||
| } | ||
| case_plan = _RUNNER._compile_case_plan(resolved_case) | ||
| self.assertEqual( |
There was a problem hiding this comment.
This asserts private dataclass field order instead of lifecycle behavior. Any unrelated field reordering will fail it, even if the contract is intact. Prefer a behavior-level assertion on finalize output or on the absence of collect side effects.
| help="Canonical CI case config YAML emitted by test_runner.", | ||
| ) | ||
| parser.add_argument( | ||
| "--feature", |
There was a problem hiding this comment.
This wrapper now has two sources of truth for transport selection: --feature / FLUXON_KV_TEST_TRANSPORT_FEATURE and scene_config.kv_transport_feature. The runner-native scene already provides canonical config, so this parallel surface can drift. Prefer one contract and fail fast on mismatch only if you absolutely need an override.
| counted=False, | ||
| ci_out={"rc": rc}, | ||
| ) | ||
| for phase in prepared_case.plan.collect_phases: |
There was a problem hiding this comment.
This removes the only explicit post-run collect hook for the CI case plan. If collect is still needed for status capture or cleanup, the contract has been dropped here and needs a replacement path.
| return closed_sdk_root | ||
|
|
||
|
|
||
| def _prepare_cargo_env(env: dict[str, str] | None) -> dict[str, str] | None: |
There was a problem hiding this comment.
This weakens the cargo wrapper contract for bundled fluxon_pyo3 runtimes. Before this change _prepare_cargo_env rewrote LD_LIBRARY_PATH to keep exactly one authoritative fluxon_pyo3.libs root; now it only sets FLUXON_PYO3_LIBS_DIR and leaves stale loader entries untouched. Rust still enforces a single loaded root (set_authoritative_bundled_ld_library_path / enforce_single_fluxon_pyo3_libs_root), so a parent shell with an older wheel path can make these wrappers load the wrong libs or fail nondeterministically.
| outcome = ctx.RUN_OUTCOME_SUCCESS | ||
| except Exception as exc: # noqa: BLE001 | ||
| error_detail = f"{type(exc).__name__}: {exc}" | ||
| finally: |
There was a problem hiding this comment.
This removes the only TEST_STACK collect hook. finalize_test_stack_case_runtime only tears down apply ids; it no longer writes the per-instance status snapshots under run_dir/logs/**, and the summary loses collect_error as well. That is a behavior regression for failed benchmark runs, not just a plan reshaping.
| return ( | ||
| "ci_top_attention_doc_page_build", | ||
| "ci_top_attention_bin_kvtest", | ||
| "ci_top_attention_cargo_fs_core", |
There was a problem hiding this comment.
No issue in this scene-id hunk; the collect regression is in the later main() hunk that removes _run_adapter_action(..., action="collect").
| ) | ||
| _write_yaml_file(run_dir / "summary.yaml", summary) | ||
|
|
||
| _run_adapter_action( |
There was a problem hiding this comment.
This removes the collect step in the infer branch as well. That branch still writes a terminal summary and then used collect to capture controller status snapshots; without it, infer cases lose the post-run observability that the adapter still treats as best-effort cleanup data.
| description="Flat index entry for Rust util crate tests." | ||
| ) | ||
| parser.add_argument( | ||
| "--case-config", |
There was a problem hiding this comment.
This should be required. If --case-config is omitted, the wrapper can fall back to whatever build_config_ext.yml is discoverable in the tree, which weakens the runner-native contract and can bind the util tests to stale endpoints.
1 participant
No description provided.