test: Integrate mq_core tests into GitHub Actions#20
Conversation
ActivePeter
left a comment
There was a problem hiding this comment.
Reviewed against AGENTS.md and the teststack design docs. I cannot request changes on this PR with the current GitHub identity, so I am leaving the blocking findings as review comments. The teststack/MQ runner changes are mostly aligned with the runner-owned CI execution model, but the repo-local skills/ tree contains private environment details and non-Fluxon operating rules and should be removed or split out.
| filtered_passthrough.append(token) | ||
| idx += 1 | ||
| for test_path in TEST_PATHS: | ||
| rc = call([python, "-u", str((Path(__file__).resolve().parents[2] / test_path)), *filtered_passthrough]) |
There was a problem hiding this comment.
This keeps the old pytest-style passthrough surface but now executes each file as a standalone script. Arguments such as -k payload or -q are forwarded to scripts that never parse them, so callers can think they selected a subset while both long MQ scripts still run. Since the previous run_pytest(...) path honored these selectors, please either reject pytest passthrough for this scene or expose an explicit finite selector for the two script entries instead of silently ignoring pytest flags.
| @@ -1,2 +1,5 @@ | |||
| deployconf_path: ../../deployment/deployconf.yaml | |||
| kv_svc_type: fluxon | |||
| etcd_address: 127.0.0.1:2379 | |||
There was a problem hiding this comment.
This moves the checked-in default test authority from the generic deployconf indirection to a concrete localhost runtime. CI does rewrite src/fluxon_py/tests/test_config.yaml with case-scoped values, but outside that prepared src_root every default import of fluxon_py.tests.test_lib now resolves to 127.0.0.1:2379 and /tmp/fluxon-example-cluster/* unless callers know to set FLUXON_TEST_CONFIG_PATH. Since repo YAMLs are examples by default and environment-specific runtime authority should be supplied separately, please keep the checked-in file as a generic example or make the local override path explicit in the runner/docs instead of committing this concrete runtime.
| "/tmp/venv/bin/python3", | ||
| "-u", | ||
| str(REPO_ROOT / "fluxon_py/tests/test_mq/test_capacity_and_auto_clean.py"), | ||
| "-k", |
There was a problem hiding this comment.
This test now protects the problematic passthrough contract: -k payload is expected to be forwarded to both standalone scripts, where it is not parsed and therefore cannot filter anything. After switching _mq_core.py away from pytest, please update this expectation to cover the intended bounded selector surface, or assert that pytest-style flags are rejected, so the silent full-run behavior does not become the stable contract.
| def _ci_runtime_wheelhouse_root(*, test_rsc_root: Path) -> Path: | ||
| return _test_stack_runtime_wheelhouse_root( | ||
| test_rsc_root=test_rsc_root, | ||
| python_abi=_TEST_STACK_DEFAULT_PYTHON_ABI, |
There was a problem hiding this comment.
This fixes the CI dependency surface only for the GitHub 3.10 job, but ci_2_virt_node.py and _prepare_ci_case() create the CI venv from the current sys.executable. On any supported local/dev runner using Python 3.11/3.12, this still points pip at python_runtime/cpython3.10/wheels; packages such as grpcio are cp-tagged, so pip install --no-index --find-links ... grpcio==1.80.0 will fail with no compatible distribution. Since the repo contract is Python >=3.10, please either derive the wheelhouse ABI from venv_python and require matching prepared artifacts, or fail fast with an explicit Python 3.10-only CI runner contract before installing.
| def _ci_runtime_offline_dependency_requirements(*, test_rsc_root: Path) -> Tuple[str, ...]: | ||
| return _offline_dependency_requirements_from_test_rsc_root( | ||
| test_rsc_root=test_rsc_root, | ||
| dependency_set_ids=("base",), |
There was a problem hiding this comment.
This makes CI dependency selection a single global base set. In this PR the only new Python workload is ci_top_attention_mq_core, and _mq_core.py now executes the two scripts directly instead of using pytest, but prepare.yaml still adds pytest to base and this path installs that same set for doc-page and Rust CI cases too. That couples unrelated runner-native scenes to one growing wheelhouse and makes future scene additions inherit unrelated tool dependencies. Please keep this as an explicit finite branch, for example derive dependency sets from the CI scene/subject/runtime contract, or drop pytest if the MQ entry no longer needs it.
| if isinstance(release_artifacts, dict): | ||
| python_wheel = release_artifacts.get("python_wheel") | ||
| if isinstance(python_wheel, str) and python_wheel.strip(): | ||
| artifact_set["release_artifacts"] = {"wheel": python_wheel} |
There was a problem hiding this comment.
This test-only rewrite hides a suite contract mismatch instead of exercising the checked-in contract. fluxon_test_stack/ci_test_list.yaml still declares release_artifacts.python_wheel and pyo3_wheel, while _parse_artifact_set() only accepts release_artifacts.wheel, so the canonical suite cannot be parsed by test_runner.py unless tests or ci_2_virt_node.py rewrite it first. Since ci_test_list.yaml is the suite contract and the repo rules prefer one canonical name for one concept, please fix the suite/parser boundary directly, or keep any legacy-name conversion in a dedicated adapter layer rather than duplicating it inside contract tests.
| { | ||
| "instance_id": instance_id, | ||
| "apply_id": _require_str(apply_id, f"CI {instance_id} apply_id"), | ||
| "instance_id": ",".join( |
There was a problem hiding this comment.
This changes the preserved runtime schema so a field named instance_id can contain a comma-joined list such as master,owner_0. That keeps deletion deduped, but it weakens the artifact contract: readers now have to know that a singular string field may encode multiple instances, while schema_version stays at 1. Please keep this structured as instance_ids: [...] (or preserve per-instance rows and dedupe by apply_id when deleting) so the cleanup artifact remains self-describing and future tooling does not need to parse display text.
|
|
||
| 使用 agent-browser 排查运行时状态, | ||
| # 业务指引 | ||
| 请你 操作 agent-browser 打开 https://10.126.126.231:8111/ 然后通过侧栏进入 HY workspace, 打开 dever_for_dev 项目 |
There was a problem hiding this comment.
This adds a personal agent skill with a concrete internal service URL and workstation path (https://10.126.126.231:8111/, /opt/store_team_dev/zyc/...) into the project repository. The PR is otherwise changing the teststack/MQ CI contract, and there is no repo-level contract or docs that make skills/ a Fluxon-owned product surface. Keeping user/session automation prompts in-tree expands the maintenance and review scope, leaks environment topology, and makes future agents load behavior that is unrelated to Fluxon. Please remove these personal/generated skill files from this PR, or add a deliberate project-level ownership contract for skills/ before landing them.
| ], | ||
| parser = argparse.ArgumentParser(add_help=False) | ||
| parser.add_argument( | ||
| "--case-config", |
There was a problem hiding this comment.
Unlike _doc_page_build.py and _bin_kvtest.py, this runner-native CI entry makes --case-config optional. If _mq_core.py is run directly, or a future runner command omits the flag, the MQ scripts still execute against whichever fluxon_py/tests/test_config.yaml is present in the checkout instead of the case-scoped etcd/cluster/shared paths emitted by test_runner. That creates two runtime authority paths for the same CI scene and makes local success/failure depend on ambient repo state. Please make --case-config required for this CI entry and reject missing config; if an ad-hoc local smoke mode is needed, expose it as a separate explicit command.
| scales: [n1_kvowner_dram_20gib] | ||
| profiles: [fluxon_tcp] | ||
|
|
||
| ci_top_attention_mq_core: |
There was a problem hiding this comment.
This adds a third top-attention runner-native CI scene, but the teststack design doc still describes the stable native dispatch set as only _bin_kvtest.py and _doc_page_build.py under section 9.1, and says ci_scene_config.yaml is handed to those two entries. Since ci_test_list.yaml is the suite contract and this PR is extending that public finite branch set, please update the design doc alongside the scene addition to include ci_top_attention_mq_core, its cluster_kv_owner runtime, and how _mq_core.py consumes the case-scoped config. Otherwise the implementation and architecture contract diverge immediately, making future CI scene work copy whichever source is stale.
| release_view_root=release_link_path, | ||
| ) | ||
|
|
||
| _prepare_ci_runtime_python_env( |
There was a problem hiding this comment.
The new CI path installs pinned runtime dependencies from the offline wheelhouse, but the subsequent Fluxon wheel install still runs plain pip install --force-reinstall <wheel> without --no-index or --no-deps. setup.py declares normal dependencies (PyYAML>=6.0, msgpack, readerwriterlock, etc.), so after this offline step the final install is still allowed to resolve from the network or drift if metadata changes or a dependency was missed in prepare.yaml. The TEST_STACK runtime path already closes this by installing offline requirements first and then installing the Fluxon wheel with --force-reinstall --no-deps; please use the same contract here so CI runtime inputs are fully bounded by the prepared test_rsc artifacts.
mq_core includes TEST_PATHS = [
"fluxon_py/tests/test_mq/test_capacity_and_auto_clean.py",
"fluxon_py/tests/test_mq/test_payload_lease_error.py",
]