Skip to content

Refactor: let the C++ unit test tree's directories say what its CMake used to - #2418

Merged
ChaoWao merged 1 commit into
hw-native-sys:mainfrom
poursoul:refactor/ut-cpp-layout-batch0
Sep 22, 2026
Merged

ChaoWao merged 1 commit into
hw-native-sys:mainfrom
poursoul:refactor/ut-cpp-layout-batch0

Conversation

@poursoul

Copy link
Copy Markdown
Collaborator

Summary

tests/ut/cpp was one 1979-line CMakeLists declaring 185 targets, with 377
${CMAKE_SOURCE_DIR}/../../../src/... paths and sixteen helpers named after
the (arch, runtime) pairs they hardcoded. Two problems beyond the size:

  • A test source said nothing about which runtime it covers — 57 files include
    bare-name headers that exist in both trees (task_id.h, runtime.h, …),
    resolved by include-path order rather than by any information in the file.
  • Most trees under src/ are compiled more than once by the product, while
    their tests were built once — a case covering one configuration and
    reporting on all looks identical to full coverage from the outside.

tests/ut/cpp now mirrors src/. The mirror path fixes the include contract
(cmake/contract.cmake states it once instead of 377 times), fixes the
runtime (a misfiled case fails to compile instead of silently taking the
other tree's headers), and fixes the compilation context (how many times a
case is built follows from where it lives).

  • Each of the 18 leaf directories ends in simpler_ut_glob_cases — a
    test_*.cpp needing nothing beyond its directory's shape is compiled and
    registered without being named anywhere.
  • simpler_ut_assert_no_orphan_cases() fails the configure if any
    test_*.cpp sits outside those 18 directories, naming the file. This is
    the failure mode the whole refactor exists to prevent — a case built by
    nothing looks identical to full coverage until someone counts.
  • Platform-case coverage (PER_ARCH / PER_RUNTIME / SINGLE) is now
    measured, not asserted: tests/lint/check_ut_cpp_axis.py preprocesses each
    translation unit and compares the combinations built against the ones the
    code actually varies over.
  • CANN stand-ins move into a static archive per arch;
    tests/lint/check_ut_cpp_stub_linkage.py guards against the weak-symbol
    trap that made 10 tests hang rather than fail when a stub was
    archive-only.
  • A discovered runtime must have its mirror directory under
    tests/ut/cpp/common/, checked at configure.
  • docs/testing/adding-a-cpp-unit-test.md replaces a recipe referencing
    four CMake variables and a source file that no longer exist.

Test plan

  • Clean configure + full parallel build: 0 undefined reference / 0
    multiple definition
  • ctest -LE requires_hardware -j8: 229/229 passed (~6.2s)
  • check_ut_cpp_axis.py, check_ut_cpp_stub_linkage.py,
    check_ut_cpp_case_naming.py: all pass
  • pytest tests/ut/py/test_swimlane_export_run_identity.py: 13 passed,
    0 skipped
  • Orphan / duplicate cross-checks against the merge-base diff: clean

229 targets (was 173), 6.2s, root CMakeLists.txt 1979 lines to 188, zero
relative ../../../src/ paths, zero unlabelled tests.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 238 files, which is 138 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1ab4ef90-9545-46dc-8e6b-89cb2b348ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 88b3205 and 07dba4f.

📒 Files selected for processing (238)
  • .github/ISSUE_TEMPLATE/bug_report.yml
  • .github/ISSUE_TEMPLATE/performance_issue.yml
  • .github/workflows/_st-deepseek-a2a3.yml
  • .github/workflows/_st-npu-a2a3.yml
  • .github/workflows/_st-npu-a5.yml
  • .github/workflows/_ut-no-hardware.yml
  • .pre-commit-config.yaml
  • docs/buffer-abi.md
  • docs/capability-survey.md
  • docs/ci.md
  • docs/design/run-completion-fence.md
  • docs/investigations/2026-08-hbg-graph-definition-single-upload.md
  • docs/remote-l3-worker-design/pr-split-and-audit-artifacts.md
  • docs/testing.md
  • docs/testing/adding-a-cpp-unit-test.md
  • docs/troubleshooting/device-error-codes.md
  • docs/troubleshooting/device-error-codes/untested.md
  • mkdocs.yml
  • src/a2a3/platform/onboard/host/device_runner.cpp
  • src/a5/platform/onboard/host/device_runner.cpp
  • src/common/host_build_graph/dep_gen_host_graph.h
  • tests/lint/check_kernel_wire_isolation.py
  • tests/lint/check_ut_cpp_axis.py
  • tests/lint/check_ut_cpp_case_naming.py
  • tests/lint/check_ut_cpp_stub_linkage.py
  • tests/st/host_build_graph_wide_dispatch/test_host_build_graph_wide_dispatch.py
  • tests/st/task_timing/task_timing_slots/test_task_timing_e2e.py
  • tests/ut/cpp/CMakeLists.txt
  • tests/ut/cpp/a2a3/CMakeLists.txt
  • tests/ut/cpp/a2a3/platform/CMakeLists.txt
  • tests/ut/cpp/a2a3/platform/test_aicpu_affinity_select.cpp
  • tests/ut/cpp/a2a3/platform/test_thread_scheduling.cpp
  • tests/ut/cpp/a2a3/runtime/CMakeLists.txt
  • tests/ut/cpp/a2a3/runtime/host_build_graph/CMakeLists.txt
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_dep_gen_host_graph.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_dispatch_table.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_graph_activation.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_hbg_submit_poison.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_hbg_task_allocator.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_hbg_tensor_access.cpp
  • tests/ut/cpp/a2a3/runtime/host_build_graph/test_hbg_tensormap.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/CMakeLists.txt
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_a2a3_fatal.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_aicore_completion_mailbox.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_args_dump.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_dep_list_pool.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_fanin_pool.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_orchestrator_fanin.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_ready_queue.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_scheduler_state.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_scope_stats_collector.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_shared_memory.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_task_allocator.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_task_state.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_task_timing_slots.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_tensormap.cpp
  • tests/ut/cpp/a2a3/runtime/tensormap_and_ringbuffer/test_wiring.cpp
  • tests/ut/cpp/a2a3/runtime/test_aicore_retirement.cpp
  • tests/ut/cpp/a5/CMakeLists.txt
  • tests/ut/cpp/a5/platform/CMakeLists.txt
  • tests/ut/cpp/a5/platform/test_aicpu_topology_fallback.cpp
  • tests/ut/cpp/a5/platform/test_host_log_off.cpp
  • tests/ut/cpp/a5/runtime/CMakeLists.txt
  • tests/ut/cpp/a5/runtime/host_build_graph/CMakeLists.txt
  • tests/ut/cpp/a5/runtime/host_build_graph/hbg_scheduler_test_support.h
  • tests/ut/cpp/a5/runtime/host_build_graph/test_graph_activation.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_legacy_terminal_record.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_report_epoch.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_resident_late_aicore_error.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_scheduler_bootstrap.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_scheduler_contracts.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_scheduler_dispatch.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_scheduler_ready.cpp
  • tests/ut/cpp/a5/runtime/host_build_graph/test_hbg_submit_poison.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/CMakeLists.txt
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_a5_fatal.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_args_dump.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_dep_list_pool.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_fanin_pool.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_orchestrator_fanin.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_rdma_completion_scheduler.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_ready_queue.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_scheduler_state.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_shared_memory.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_task_allocator.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_task_state.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_tensormap.cpp
  • tests/ut/cpp/a5/runtime/tensormap_and_ringbuffer/test_wiring.cpp
  • tests/ut/cpp/a5/runtime/test_aicore_completion_mailbox.cpp
  • tests/ut/cpp/cmake/contract.cmake
  • tests/ut/cpp/cmake/runtimes.cmake
  • tests/ut/cpp/cmake/ut_test.cmake
  • tests/ut/cpp/common/CMakeLists.txt
  • tests/ut/cpp/common/hierarchical/CMakeLists.txt
  • tests/ut/cpp/common/hierarchical/test_chip_run_lane.cpp
  • tests/ut/cpp/common/hierarchical/test_control_copy_request.cpp
  • tests/ut/cpp/common/hierarchical/test_orchestrator.cpp
  • tests/ut/cpp/common/hierarchical/test_pending_run_queue.cpp
  • tests/ut/cpp/common/hierarchical/test_pipeline_contract.cpp
  • tests/ut/cpp/common/hierarchical/test_pipeline_contract_loader.cpp
  • tests/ut/cpp/common/hierarchical/test_remote_endpoint.cpp
  • tests/ut/cpp/common/hierarchical/test_remote_wire.cpp
  • tests/ut/cpp/common/hierarchical/test_ring.cpp
  • tests/ut/cpp/common/hierarchical/test_scheduler.cpp
  • tests/ut/cpp/common/hierarchical/test_scope.cpp
  • tests/ut/cpp/common/hierarchical/test_tensormap.cpp
  • tests/ut/cpp/common/host_build_graph/CMakeLists.txt
  • tests/ut/cpp/common/host_build_graph/support/scheduler_drain_a5_stubs.cpp
  • tests/ut/cpp/common/host_build_graph/support/stall_dump_level_a2a3_stubs.cpp
  • tests/ut/cpp/common/host_build_graph/support/stall_dump_level_a5_stubs.cpp
  • tests/ut/cpp/common/host_build_graph/test_async_poll_phase_accumulator.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_async_wait_init.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_bind_ledger.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_core_tracker.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_ed_qualification.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_async_submit.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_cache.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_definition_arena.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_ed_qualification.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_recording_bounds.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_graph_submit_failure.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_mailbox_init.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_ready_queue_seed.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_scheduler_drain.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_self_relative_ptr.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_slot_claim.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_sm_compaction.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_stall_dump_level.cpp
  • tests/ut/cpp/common/host_build_graph/test_hbg_task_id.cpp
  • tests/ut/cpp/common/host_build_graph/test_native_run_acceptance.cpp
  • tests/ut/cpp/common/log/CMakeLists.txt
  • tests/ut/cpp/common/log/test_host_log_cross_dso.cpp
  • tests/ut/cpp/common/log/test_host_log_dso_unload.cpp
  • tests/ut/cpp/common/log/test_host_log_nonblocking.cpp
  • tests/ut/cpp/common/log/test_host_log_unbound.cpp
  • tests/ut/cpp/common/log/test_sim_device_log.cpp
  • tests/ut/cpp/common/platform/CMakeLists.txt
  • tests/ut/cpp/common/platform/chip_swimlane_run_export_fixture.h
  • tests/ut/cpp/common/platform/spin_hint_selector_check.cpp
  • tests/ut/cpp/common/platform/support/profiling_copy_fault.cpp
  • tests/ut/cpp/common/platform/support/profiling_copy_fault.h
  • tests/ut/cpp/common/platform/test_acl_hal_device.cpp
  • tests/ut/cpp/common/platform/test_args_dump_collector.cpp
  • tests/ut/cpp/common/platform/test_args_dump_run_identity.cpp
  • tests/ut/cpp/common/platform/test_buffer_pool_manager.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_aicore.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_aicore_accounting.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_collector.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_handoff_accounting.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_run_export.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_run_terminal.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_run_terminal_consistency.cpp
  • tests/ut/cpp/common/platform/test_chip_swimlane_run_terminal_transport.cpp
  • tests/ut/cpp/common/platform/test_dep_gen_collector_aicpu.cpp
  • tests/ut/cpp/common/platform/test_dep_gen_run_identity.cpp
  • tests/ut/cpp/common/platform/test_device_fault_cross_dso.cpp
  • tests/ut/cpp/common/platform/test_device_fault_monitor.cpp
  • tests/ut/cpp/common/platform/test_device_health_state.cpp
  • tests/ut/cpp/common/platform/test_device_phase_capture.cpp
  • tests/ut/cpp/common/platform/test_execution_mode_latch.cpp
  • tests/ut/cpp/common/platform/test_file_marker_handshake.cpp
  • tests/ut/cpp/common/platform/test_host_api.cpp
  • tests/ut/cpp/common/platform/test_host_phase_records.cpp
  • tests/ut/cpp/common/platform/test_kernel_args_helper.cpp
  • tests/ut/cpp/common/platform/test_kernel_entry_validation.cpp
  • tests/ut/cpp/common/platform/test_kernel_execution_state.cpp
  • tests/ut/cpp/common/platform/test_kernel_persistent_args.cpp
  • tests/ut/cpp/common/platform/test_memory_allocator.cpp
  • tests/ut/cpp/common/platform/test_onboard_device_log_level.cpp
  • tests/ut/cpp/common/platform/test_orch_so_file.cpp
  • tests/ut/cpp/common/platform/test_pmu_collector.cpp
  • tests/ut/cpp/common/platform/test_pmu_run_identity.cpp
  • tests/ut/cpp/common/platform/test_profiler_base.cpp
  • tests/ut/cpp/common/platform/test_profiler_device_engine.cpp
  • tests/ut/cpp/common/platform/test_region_instance_view.cpp
  • tests/ut/cpp/common/platform/test_retained_temp_bump.cpp
  • tests/ut/cpp/common/platform/test_run_completion_fence.cpp
  • tests/ut/cpp/common/platform/test_run_drain_decision.cpp
  • tests/ut/cpp/common/platform/test_run_outcome_decision.cpp
  • tests/ut/cpp/common/platform/test_run_shadow_consumer_evidence.cpp
  • tests/ut/cpp/common/platform/test_run_stream_pair.cpp
  • tests/ut/cpp/common/platform/test_run_terminal_record.cpp
  • tests/ut/cpp/common/platform/test_runtime_orch_so.cpp
  • tests/ut/cpp/common/platform/test_runtime_timeout_config.cpp
  • tests/ut/cpp/common/platform/test_runtime_workers_boundary.cpp
  • tests/ut/cpp/common/platform/test_runtime_workers_publication.cpp
  • tests/ut/cpp/common/platform/test_scope_stats_collector.cpp
  • tests/ut/cpp/common/platform/test_sim_run_completion.cpp
  • tests/ut/cpp/common/platform/test_teardown_recorder.cpp
  • tests/ut/cpp/common/platform/test_worker_chip_message_queue.cpp
  • tests/ut/cpp/common/platform/test_worker_chip_orch_comm.cpp
  • tests/ut/cpp/common/platform/test_worker_chip_orch_endpoint.cpp
  • tests/ut/cpp/common/platform_comm/CMakeLists.txt
  • tests/ut/cpp/common/platform_comm/test_comm_lifecycle.cpp
  • tests/ut/cpp/common/runtime_status/CMakeLists.txt
  • tests/ut/cpp/common/runtime_status/test_error_code_names.cpp
  • tests/ut/cpp/common/support/device_fault_client.cpp
  • tests/ut/cpp/common/support/host_log_consumer.cpp
  • tests/ut/cpp/common/support/host_log_unload_consumer.cpp
  • tests/ut/cpp/common/support/native_run_execution_peer.h
  • tests/ut/cpp/common/task_interface/CMakeLists.txt
  • tests/ut/cpp/common/task_interface/test_buffer.cpp
  • tests/ut/cpp/common/task_interface/test_call_config.cpp
  • tests/ut/cpp/common/task_interface/test_callable_scalar_count.cpp
  • tests/ut/cpp/common/task_interface/test_child_memory.cpp
  • tests/ut/cpp/common/task_interface/test_chip_callable_upload_immutable.cpp
  • tests/ut/cpp/common/task_interface/test_chip_max_tensor_args.cpp
  • tests/ut/cpp/common/task_interface/test_kernel_invocation_header.cpp
  • tests/ut/cpp/common/tensormap_and_ringbuffer/CMakeLists.txt
  • tests/ut/cpp/common/tensormap_and_ringbuffer/test_dep_gen_replay.cpp
  • tests/ut/cpp/common/tensormap_and_ringbuffer/test_scope_deadlock_detection.cpp
  • tests/ut/cpp/common/tensormap_and_ringbuffer/test_trb_runtime_temp_buffer.cpp
  • tests/ut/cpp/common/utils/CMakeLists.txt
  • tests/ut/cpp/common/utils/test_device_arena.cpp
  • tests/ut/cpp/common/utils/test_elf_build_id.cpp
  • tests/ut/cpp/common/utils/test_fatal_shutdown_latch.cpp
  • tests/ut/cpp/common/utils/test_thread_completion_gate.cpp
  • tests/ut/cpp/common/worker/CMakeLists.txt
  • tests/ut/cpp/common/worker/test_native_run_execution.cpp
  • tests/ut/cpp/stubs/test_stubs.cpp
  • tests/ut/cpp/support/CMakeLists.txt
  • tests/ut/cpp/support/assert_stubs.cpp
  • tests/ut/cpp/support/cache_maintenance_stub.cpp
  • tests/ut/cpp/support/device_time_stub.cpp
  • tests/ut/cpp/support/dlog_pub.h
  • tests/ut/cpp/support/hbg_orch_stubs.cpp
  • tests/ut/cpp/support/kernel_args/acl/acl.h
  • tests/ut/cpp/support/kernel_args/acl/error_codes/rt_error_codes.h
  • tests/ut/cpp/support/kernel_args/runtime/rt.h
  • tests/ut/cpp/support/pipeline_contract_runtime.cpp
  • tests/ut/cpp/support/platform_regs_stub.cpp
  • tests/ut/cpp/support/unified_log_stubs.cpp
  • tests/ut/cpp/support/weak_link_placeholders.cpp
  • tests/ut/py/support/runtime_publication_launch_probe.cpp
  • tests/ut/py/test_buffer.py
  • tests/ut/py/test_runtime_publication_launch.py
  • tests/ut/py/test_swimlane_export_run_identity.py
  • tests/ut/py/test_ut_cpp_checkers.py

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@poursoul
poursoul force-pushed the refactor/ut-cpp-layout-batch0 branch 2 times, most recently from df56e9a to d47887d Compare September 21, 2026 12:07

@zhusy54 zhusy54 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

目录镜像、统一 CMake 注册及按符号组拆分 stub 静态库的方向合理。此前在 df56e9a 上完成了干净配置、完整构建和 229 个 CTest target 的验证,均通过,三个新增检查器也通过;但最小复现发现覆盖检查器存在漏报,迁移还丢失了 retirement 测试已有的超时保护。

发布前已核对最新 d47887d:预处理失败的退出码检查已补上,runtime 列表也已改为动态发现,因此不再将这两项作为未修复问题。四条 inline comments 已按最新代码调整;此前的 229-target 测试结果不代表已重新测试最新提交。

建议修复 inline 问题,并补充检查器自身的回归测试,至少覆盖:差异仅存在于主 .cpp、链接对象库中的差异、带空格/引号的编译参数、预处理失败,以及 /runtime/ 用例缺失一个 runtime 组合。当前业务用例通过不能替代这些检查器失败路径的验证。

文档也请一并统一:

  • docs/testing/adding-a-cpp-unit-test.md 的 “Do not add a _case helper with an arch or runtime in its name” 与前文及实际 hbg_case/a2a3_hbg_case 用法矛盾,需要明确禁止的是哪类重复 helper。
  • “There is no default” 与 common/platform 的 glob 使用 platform_single_case 不一致;请区分显式声明的要求与自动发现的默认行为。
  • 清理最终树中已不存在的 platform_api_stubs.cpp 引用。

基于这些覆盖保障与测试行为保留问题,建议 request changes。

f"reaches cannot be read. The compile line was rewritten to preprocess:\n\n"
f" {' '.join(argv)}\n\n{result.stderr.strip()}"
)
opened = set()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 将主生产 .cpp 和链接对象库的编译单元纳入覆盖分析

这里仅从 -H 输出收集路径,但 -H 不包含正在编译的主 .cpp。本地最小复现中,a2a3/platform/probe.cpp 与 a5/platform/probe.cpp 实现不同,只构建 a2a3;differs_across() 能确认两份源文件不同,但 files_opened() 返回空集,find_gaps() 最终返回无缺口。

另外,find_gaps() 只遍历直接含 test_*.cpp 的 target,未追踪测试链接的 OBJECT libraries,其实现编译单元同样未被纳入。这会漏掉仅存在于实现文件、而非头文件中的架构/runtime 差异。请至少将 entry['file'] 纳入输入,并沿实际目标依赖纳入对象库编译上下文,补充对应的失败回归测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

前半已修,后半实测会引入误报,想和你确认一下再定 —— 详细如下(07dba4f4)。

主 .cpp:已纳入

files_opened 现在显式把 entry["file"] 加进结果(check_ut_cpp_axis.py:256)。你说的现象确认了,-H 输出里一个 .cpp 都没有:

主 .cpp        : a2a3/platform/sim/host/profiling_copy.cpp
是否在 opened 里: False
opened 中 .cpp 数: 0

而这个文件正是 @arch@ 展开出来的两份不同实现(inner_platform_regs.cpp 同样),所以这半确实是真漏报。回归测试 test_a_difference_only_in_a_cpp_body_is_still_a_gap,对修复前的代码失败。

链接对象库:按你的方案实现后出现误报,暂未纳入

我先照你说的做了 —— 读 CMakeFiles/<target>.dir/link.txt 取出链接的 target,把它们的编译单元并进该 case 的 opened。结果检查器立刻报出一条:

common/host_build_graph/test_hbg_task_id.cpp
    missing: a5    built: a2a3

定位后是误报。该 case 只 include src/common/host_build_graph/task_id.h(arch 无关),自身 TU 沿 arch 无任何变化;触发判定的是它链接的 a2a3_hbg_objs 里的一个头:

自身 TU 中沿 arch 变化的: 无
a2a3_hbg_objs 中沿 arch 变化的: 1 个, 例如 ['a2a3/platform/include/common/platform_config.h']

它测的是 TaskId 句柄的位域打包,完全不碰 platform_config.h。

问题在于 <arch>_<runtime>_objs 被目录内几乎所有 case 共享,且无论哪个 case 在测什么,它本身都沿 arch 变化。把它的差异记到每个链接者头上,等价于「凡链接了 arch 特有对象库的 case 都必须建两个 arch」—— 现有 case 大多已是双 arch,所以只有显式 ARCHS a2a3 的这一个暴露出来,但规则本身是过严的:链接进来解析符号 ≠ 覆盖了那段代码。

所以我把判定边界定在「case 自己 target 的编译单元」:SOURCES 里刻意列进来的 src/ 实现是它选择覆盖的对象,链接依赖不是。这个决定固化成了测试 test_a_difference_in_a_linked_object_library_is_not_the_cases_gap。

附带一点:link.txt 只有 Makefiles generator 会写,Ninja 不写,所以那条路径还需要一个 generator 无关的来源(File API)才稳妥。

如果你认为对象库里的差异仍应计入,我想听下你对 test_hbg_task_id 这类的预期 —— 是应当把它也改成双 arch 构建,还是走 WAIVERS?两种我都可以做,只是想避免让 WAIVERS 变成「凡收窄 ARCHS 必登记」的常规入口。

检查器自测

按你的要求补了 tests/ut/py/test_ut_cpp_checkers.py(11 条),覆盖你列的五项:主 .cpp 差异、对象库差异的归属、带空格/引号的编译参数、预处理失败、<arch>/runtime/ 缺 runtime 组合;另含 arguments 字段优先、宏转义、无 target 条目的两种报法、runtime 从 build_config.py 发现。每条都逐一验证过能在修复前的代码上失败,不是只在当前代码上通过。

CI 的 pytest tests/ut 已自动收集,未改 workflow。

Comment thread tests/lint/check_ut_cpp_axis.py Outdated

def preprocess_command(command: str) -> list[str]:
"""The compile line, rewritten to preprocess and list every file it opens."""
argv, tokens, index = [], command.split(), 0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 按 shell quoting 规则解析 compile_commands 中的 command

command.split() 会破坏 CMake 在含空格路径或宏值上生成的引号/转义。例如一个合法的 -I".../include space" 参数会变成两个错误参数。已用最小编译记录验证:原命令编译成功,检查器重放预处理却失败。

最新提交已正确拒绝非零预处理退出码,因此原来的静默通过问题已修复;但合法构建现在仍会因这里的参数解析而无法通过检查。建议优先使用 compilation database 的 arguments 字段;读取 command 时使用与当前 POSIX 工具链匹配的 shlex.split,并用相同解析结果提取 -o,补充带空格/引号参数的回归测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修复(07dba4f4)。

新增 command_argv()(check_ut_cpp_axis.py:195):优先取 compilation database 的 arguments 字段,只有落到 command 时才解析,且用 shlex.split 而非 .split();-o 的提取和错误信息里的命令行回显都改走同一份 argv(后者用 shlex.join)。

你指出的两种破坏都复现了:

原始:  c++ -DNAME=\"a2a3sim\" -I"/path/with space/include" -o foo.o -c foo.cpp
.split()     -> ['c++', '-DNAME=\\"a2a3sim\\"', '-I"/path/with', 'space/include"', ...]
shlex.split  -> ['c++', '-DNAME="a2a3sim"',     '-I/path/with space/include',      ...]

也确认了你关于严重性的推断:因为上一版已经把非零退出码改成硬失败,这个解析问题现在不再是静默漏报,而是会直接挡住合法构建。

回归测试三条:

  • test_an_argument_with_a_space_survives_the_replay
  • test_the_arguments_field_is_preferred_when_present
  • test_a_define_keeps_the_quotes_the_shell_would_have_removed

第三条起初是拿 -E 替身验证的,回退代码后它仍然通过 —— -E 不做语法检查,\"x\" 的宏体预处理不报错。改成直接断言 command_argv() 的输出后三条都能抓到修复前的代码:

回退 command_argv 后
FAILED test_an_argument_with_a_space_survives_the_replay
FAILED test_the_arguments_field_is_preferred_when_present
FAILED test_a_define_keeps_the_quotes_the_shell_would_have_removed

Comment thread tests/lint/check_ut_cpp_axis.py Outdated
if parts[0] in ARCHES:
# The arch is fixed by the directory. Only the platform tree under it is
# compiled once per runtime.
return (False, len(parts) > 1 and parts[1] == "platform")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 为直接位于 /runtime/ 的用例保留 runtime 覆盖轴

这里仅当 parts[1] == 'platform' 时开启 runtime 轴,使 a2a3/runtime/test_aicore_retirement.cpp 和 a5/runtime/test_aicore_completion_mailbox.cpp 都得到 (False, False)。但这两个目录的 CMake 明确把公共用例分别构建为 HBG/TMR 两个版本,它们并不属于某一个固定 runtime。

已用实际编译记录验证:只保留 HBG retirement target、移除其 TMR sibling 后,find_gaps() 仍不报告缺口。请区分直接位于 <arch>/runtime/ 的公共用例与位于 <arch>/runtime/<runtime>/ 的专属用例,并补一个缺失 runtime 组合的回归测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修复(07dba4f4)。

applicable_axes 现在区分两种位置(check_ut_cpp_axis.py:143):<arch>/runtime/ 下的公共用例保留 runtime 轴,只有 <arch>/runtime/<runtime>/ 才连 runtime 一起固定。

if len(parts) > 1 and parts[1] == "runtime":
    return (False, not (len(parts) > 2 and parts[2] in RUNTIMES))

你的判断与目录的 CMake 一致:a2a3/runtime/CMakeLists.txt 的 a2a3_runtime_case 就是 hbg_case + tmr_case 两次,glob 也走它,所以这一层的用例本来就是每 runtime 各建一份。

回归测试补了两条,tests/ut/py/test_ut_cpp_checkers.py:

  • test_a_case_directly_under_arch_runtime_keeps_its_runtime_axis —— 只建 hbg 一份时必须报缺 tmr
  • test_a_case_under_arch_runtime_runtime_has_no_runtime_axis —— 下一层目录固定了 runtime,不该报

前者对修复前的代码确实失败:

回退 applicable_axes 后
FAILED test_a_case_directly_under_arch_runtime_keeps_its_runtime_axis

hbg_case(test_aicore_retirement.cpp ARCHS a2a3 NO_OBJS
SOURCES ${SIMPLER_SRC}/a2a3/runtime/host_build_graph/aicore/aicore_executor.cpp
${RETIREMENT_SOURCES}
INCLUDES ${RETIREMENT_INCLUDES})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] 保留两个 retirement target 原有的 TIMEOUT 20

merge-base 的两个 retirement target 均设置了 TIMEOUT 20,迁移后的两个声明没有保留该属性;simpler_ut_runtime_case() 也不接受或转发 TIMEOUT。已从迁移后的 CTest JSON 元数据确认该属性缺失。

这些测试包含等待断言之后的 worker.join(),协议回归时工作线程可能无法退出。丢失超时会让常规本地 ctest 长期挂住,并把 CI 的兜底从原来的 20 秒放宽到命令行默认的 300 秒。请让 runtime helper 支持并转发 TIMEOUT,并给 HBG/TMR 两个 retirement target 恢复 20 秒限制。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修复(07dba4f4)。

simpler_ut_runtime_case 现在接受并转发 TIMEOUT(cmake/ut_test.cmake:164 的单值参数列表,此前为空串),两个 retirement 声明恢复 TIMEOUT 20(a2a3/runtime/CMakeLists.txt:33,37)。

从 CTest 元数据复核,与 merge-base 一致:

test_host_log_nonblocking:        8.0
test_a2a3_hbg_aicore_retirement: 20.0
test_a2a3_tmr_aicore_retirement: 20.0

顺带确认过丢失范围就是这两个:merge-base 全树只有两处 TIMEOUT(retirement_test 的 20 与 test_host_log_nonblocking 的 8),后者走 simpler_ut_add_target 所以一直保留着 —— 根因正如你指出的,只在 runtime_case 不转发这一个 helper 上。

@poursoul
poursoul force-pushed the refactor/ut-cpp-layout-batch0 branch from d47887d to 07dba4f Compare September 21, 2026 13:04
… used to

tests/ut/cpp was one 1979-line CMakeLists declaring 185 targets, with 377
`${CMAKE_SOURCE_DIR}/../../../src/...` paths and sixteen helpers named after
the (arch, runtime) pairs they hardcoded. Two things were wrong with it beyond
the size. A test source says nothing about which runtime it covers — 57 of
them include bare-name headers that exist in both trees, so `task_id.h`
resolved by include-path order and the author picked a runtime with no
information. And most trees under src/ are compiled more than once by the
product, while their tests were built once, which looks identical from the
outside: a case covering one configuration and reporting on all is a passing
test.

The tree now mirrors src/. The mirror path fixes the include contract, so
cmake/contract.cmake states it once instead of 377 times; it fixes the runtime,
so a misfiled case fails to compile rather than silently taking the other
tree's headers; and it fixes the compilation context, so how many times a case
is built follows from where it lives.

Each of the eighteen directories ends in simpler_ut_glob_cases. A test_*.cpp
that needs nothing beyond its directory's shape is compiled and registered
without being named anywhere — declaring one by hand is what removes it from
the sweep, so there is no list of exceptions to maintain. Cases sharing a shape
are declared as a group, which says they are one kind rather than leaving the
next author to notice a pattern and copy a line. No bare add_executable
remains: one also fails to record the case file it consumes, so the glob would
build a second target from the same source.

A case file outside those eighteen directories is built by nothing, and that
is silent in every direction — the tree configures, the build succeeds, and
ctest reports what registered, so the suite stays green and smaller by exactly
the coverage nobody can see is gone. Neither other check reaches it, since both
read the targets that exist and an unbuilt case has none.
simpler_ut_assert_no_orphan_cases() compares a scan of the tree against the
cases targets recorded consuming, and fails the configure naming any file left
over. A directory the configuration did not descend into registers its cases as
skipped rather than being reported.

How many times a platform case is built is now stated, never defaulted:
PER_ARCH, PER_RUNTIME, both, or SINGLE, and omitting all four is a configure
error. A silent default is what turns an unjudged case into a single build that
afterwards looks deliberate. tests/lint/check_ut_cpp_axis.py decides the answer
by measurement — it preprocesses each translation unit, reads which trees the
compiler opened, and compares the combinations built against the ones the code
varies over, as cells of the product rather than axis by axis. That found ten
cases covering one configuration, including four whose comments asserted the
arches agreed when kernel_args.h, platform_config.h and platform_regs.h each
differ between them.

The eighteen case and support files added under the old layout take their
mirror positions here, and the axis check judges them on arrival rather than
letting the glob default them to a single build. Ten of the thirty-three
targets they expand to are a5 configurations the old declarations did not
build: the chip_swimlane accounting, consistency and transport cases reach
src/<arch>/platform, and the workers publication case reaches that arch's
allocator. All ten pass. One case does not get its a5 cells —
test_chip_swimlane_run_export compares against a golden captured on a2a3,
carrying that capture's own clock_freq_hz and platform, so an a5 build has
nothing correct to compare against until an a5 golden is captured. The writer
itself is correct there, which its five siblings on the same two sides cover;
WAIVERS records this as a gap to close rather than a combination that cannot
exist.

The CANN stand-ins move into a static archive per arch, one file per group of
symbols. A member is pulled only to resolve a symbol nothing else defined, so a
case takes the real unified_log_host.cpp or device_time.cpp by naming it and
says nothing about the stub. One symbol does not work that way: the tmr runtime
defines get_sys_cnt_aicpu weakly as `return 0`, a weak definition satisfies the
reference, the archive is never searched, and ten tests hung on a 500 ms
backstop counting ticks that never advance.
tests/lint/check_ut_cpp_stub_linkage.py holds that line, because the failure is
a hang and ctest only reaches it by timing out.

Runtimes are discovered from their build_config.py, which gives a new one the
per-runtime cases for free — and nothing else, since its own test directories
and object libraries do not exist yet. Configuring succeeded and the build then
failed on a bare-name header resolved into a directory that was never created,
reported against a platform file the author never opened. A discovered runtime
must now have its mirror directory under tests/ut/cpp/common/, checked at
configure and answered with what to create. Turning a short tag back into a
directory name matches against that discovered set and fails on no match,
rather than an else branch that answered tensormap_and_ringbuffer for every tag
it had not been taught.

CI runs both checks after ctest — they read the build rather than run it, and
catch what a green ctest cannot. Each decides on the tool's exit status rather
than on its output: a preprocessor run that did not succeed opens an unknown
subset of files, and an unknown subset reads as a case reaching neither tree,
which is the very gap the check looks for reported as absent.

Measuring which trees a case reaches has to see what the case compiles, and
`-H` lists the headers the preprocessor opened rather than the source it was
handed — so a tree whose two copies differ only in a .cpp body was invisible,
and the axis it varies over read as collapsed. The unit's own source now
counts. The object libraries a case links do not: they are shared by most
cases in their directory and differ along the arch axis whatever any one case
does, so charging those differences to each case would require a case whose
whole subject is an arch-agnostic handle to build for both arches because
something it links but never calls differs. What a case compiles into its own
target is what it chose to cover.

Replaying a compile line also means un-quoting it, since a compilation
database writes a shell line: splitting on whitespace turns a legal
`-I"/path/with space"` into two wrong arguments and leaves `-DNAME=\"x\"` with
the backslashes the shell would have removed. The already-split `arguments`
field is taken when the database carries it, and `command` goes through
shlex otherwise.

A case directly under `<arch>/runtime/` covers the parts every runtime
carries, and its directory builds it once per runtime — only
`<arch>/runtime/<runtime>/` fixes the runtime as well. Reading "not platform"
as "one runtime" dropped the axis for exactly the cases that do repeat along
it, so a missing runtime combination there went unreported.

`common/platform/` no longer defaults the axis for a file nobody declared.
Everywhere else the directory fixes the answer and its glob can supply it;
there both axes are open at once, so a glob choosing SINGLE for an
unrecognised case handed it the single build the keyword exists to make
deliberate. The fifteen cases that were being swept up say SINGLE themselves,
which check_ut_cpp_axis.py then verifies, and an undeclared file is refused by
name.

`simpler_ut_runtime_case` forwards TIMEOUT, which it previously neither
accepted nor passed on. The two AICore retirement cases wait on a retirement
and then join the worker, so a protocol regression leaves the assertion
unreached and the thread unjoined — a hang, not a failure. Their twenty-second
limit had become ctest's default of three hundred.

The checkers now have their own failure-path tests. Running one against the
real tree proves it passes, not that it would still fail when it should, and a
checker that quietly stops finding gaps looks exactly like a tree that has
none. Each test plants one gap in a synthetic src/ tree and asserts the
checker finds exactly it; every one of them fails against the code before
this commit.

A target's name is likewise the CMake tree's to assemble, from the case file
and the combination it is built for, so a consumer outside ctest that spells
one is a second implementation of that rule with nothing tying it to the
first. The swimlane roundtrip was such a consumer, and it failed in the shape
this refactor exists to remove: the two spellings drift, pytest finds no file,
and a test that reports "not built" by skipping goes quiet — eight
parametrisations skipped, exit 0, nothing red. PYUT_EXPORT records each build's
path from $<TARGET_FILE:...> keyed by its combination, and the pytest looks the
binary up instead of naming it. The manifest is written at generate time, which
keeps two questions apart: a combination absent from it is a declaration that
lost an axis and fails, while a listed path that does not exist yet is an
unbuilt tree and skips.

The four remaining hand-copied runtime lists in the workflows now ask
discover_runtimes, so adding a runtime needs no edit outside src/, its own unit
test directories, and the two issue-template dropdowns a GitHub form cannot
compute. Each checks the answer is non-empty before using it: `set -e` does not
fire on a command substitution in a `for` list, so an empty one would run the
body zero times and pass the step having compiled or asserted nothing.

docs/testing/adding-a-cpp-unit-test.md replaces a recipe whose four CMake
variables and one source file had all ceased to exist.

229 targets, 4.9s, zero bare targets, zero unlabelled tests, zero relative
paths into src/, root CMakeLists 1979 lines to 191.
@poursoul

Copy link
Copy Markdown
Collaborator Author

谢谢检视,逐条都复现了。已推 07dba4f,四条 inline 各自回复在对应线程里,这里汇总文档三条和覆盖保障。

文档三条:已修,其中两条比意见里描述的范围更大

_case helper 命名 —— 你说的矛盾成立。树里现有 19 个 helper,其中 8 个正是 a2a3_hbg_case / a5_tmr_case 这类带 arch+runtime 的。原文想禁的是 merge-base 那 16 个按 (arch, runtime) 对命名的全局 simpler_ut_* helper,和目录内的局部 macro 是两回事。已改为明确前者。

"There is no default" —— 不只是文档与实现不一致,实际后果更实。实测往 common/platform/ 丢一个 case:

configure 无报错,静默建成 1 个 test_axis_probe(SINGLE)

simpler_ut_platform_case 的 FATAL_ERROR 只对手写声明有效,simpler_ut_glob_cases(platform_single_case) 直接把 SINGLE 注进去绕过了它 —— 也就是本 PR 宣称消灭的「未经判断的 case 静默建一次」,在最需要判断轴的那个目录里仍然成立。

所以没有只改文档:common/platform/ 的 glob 现在点名拒绝未声明的文件(common/platform/CMakeLists.txt:435,451),原先被静默扫入的 15 个 case 显式写成 simpler_ut_cases(platform_single_case ...)(第 410 行),由 axis 检查器验证这些 SINGLE 是否成立。目标数不变,仍 229。文档同步说明了「其他目录有默认是正当的(位置固定了答案),common/platform/ 是唯一两轴全开因而唯一拒绝猜测的目录」。

platform_api_stubs.cpp —— 比「最终树中已不存在」更严重:merge-base 里也没有这个文件,当时叫 tests/ut/cpp/stubs/test_stubs.cpp(190 行)。所以这是个虚构文件名,读者 grep 不到任何东西。而且不止文档一处,代码注释里还有 3 处(support/platform_regs_stub.cpp 两处、common/host_build_graph/CMakeLists.txt 一处)。4 处已全部改为指向真实文件,现在 grep -rn platform_api_stubs tests/ docs/ 为空。

覆盖保障

tests/ut/py/test_ut_cpp_checkers.py,11 条,覆盖你列的五项全部:主 .cpp 差异、对象库差异的归属、带空格/引号的编译参数、预处理失败、<arch>/runtime/ 缺 runtime 组合。另加 arguments 字段优先、宏转义、无 target 条目的两种报法、runtime 从 build_config.py 发现。

每条都逐一回退对应修复验证过能失败,不是只在当前代码上通过。有一条起初无效(宏转义那条用 -E 替身验证,回退后仍通过,因为 -E 不做语法检查),改成直接断言 command_argv() 输出后才真正有效。

一处需要澄清

此前的 229-target 测试结果不代表已重新测试最新提交

提醒本身合理。补充说明:d47887dd 和现在的 07dba4f 我都跑过全套,不是沿用旧结果。07dba4f4 上:

ctest              229/229 passed
TIMEOUT            retirement ×2 = 20s, host_log_nonblocking = 8s
axis / stub / naming   exit 0
pyut               2603 passed(含新增 11 条)
pre-commit(改动文件)  全 Passed

未采纳的一点

inline #1 的后半(把链接对象库的编译单元纳入)按你的方案实现后会误报 test_hbg_task_id,理由和实测数据写在那条线程里,想听下你的意见再定。其余七条已全部按检视修复。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants