odb: migrate swap_master test from Tcl to C++#10732
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the integration test swap_master.tcl with a comprehensive C++ unit test TestSwapMasterLeaf.cpp to verify the leaf-cell master swap behavior. The reviewer's feedback focuses on improving the robustness of the new unit test by moving the fixture setup from the constructor to the SetUp() method to ensure proper initialization, and adding null-pointer assertions for dbInst::create and findITerm to prevent potential segmentation faults.
Replace the swap_master.tcl integration test with a focused C++ unit test, TestSwapMasterLeaf, covering dbInst::swapMaster(dbMaster*) (the leaf-cell master swap). The .tcl test only printed the post-swap master name and diffed it against a golden log. The unit test asserts the documented contract directly with semantic checks: - swapping to a master with an identical MTerm signature returns true, changes the master, and preserves iterm net connections (remapped by MTerm name); - swapping to an incompatible signature returns false and leaves the instance untouched (a failure path the .tcl test never exercised). Built on SimpleDbFixture, so it needs no input files. Fixture setup lives in SetUp() and null-pointer assertions guard dbInst::create and findITerm. Registered in both CMake (cpp/CMakeLists.txt) and Bazel (cpp/BUILD + the cpp_tests suite in test/BUILD). Removed swap_master.tcl and swap_master.ok and de-registered them from both build systems. Binding-level coverage of the swapMaster command remains in test_inst.tcl / test_inst.py. The existing TestSwapMaster.cpp covers the distinct dbModInst::swapMaster(dbModule*) overload. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
openroad-ci
force-pushed
the
test-swap-master-leaf-cpp
branch
from
183c2fd to
e8885c3
Compare
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the legacy integration test swap_master with a robust C++ unit test suite (TestSwapMasterLeaf.cpp) using GoogleTest to verify the leaf-cell master swap API (dbInst::swapMaster). The new tests assert that swapping to a compatible master preserves net connections by MTerm name, and that swapping to an incompatible master is rejected without mutating the instance. Build configurations in Bazel and CMake have been updated accordingly. I have no feedback to provide.
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Replace the swap_master.tcl integration test with a focused C++ unit test, TestSwapMasterLeaf, covering dbInst::swapMaster(dbMaster*) (the leaf-cell master swap).
The .tcl test only printed the post-swap master name and diffed it against a golden log. The unit test asserts the documented contract directly with semantic checks:
Built on SimpleDbFixture, so it needs no input files. Registered in both CMake (cpp/CMakeLists.txt) and Bazel (cpp/BUILD + the cpp_tests suite in test/BUILD).
Removed swap_master.tcl and swap_master.ok and de-registered them from both build systems. Binding-level coverage of the swapMaster command remains in test_inst.tcl / test_inst.py. The existing TestSwapMaster.cpp covers the distinct dbModInst::swapMaster(dbModule*) overload.