Skip to content

Add new Tcl and C++ test cases#294

Open
openroad-ci wants to merge 38 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-sta-test-by-opus
Open

Add new Tcl and C++ test cases#294
openroad-ci wants to merge 38 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-sta-test-by-opus

Conversation

@openroad-ci
Copy link
Collaborator

Summary

  • Adds comprehensive C++ and Tcl regression test suites across all 12 STA modules — 6,159 tests, 100% pass rate
  • Strengthens all C++ assertions with meaningful checks, removes empty test bodies
  • Adapts all tests to the upstream rel_3.0 API (Corner → Scene/Mode, enum class changes, command renames, etc.)

Test Coverage by Module

Module C++ Tests Tcl Tests Total
search 1,788 74 1,862
liberty 1,005 38 1,043
sdc 870 40 910
network 577 26 603
dcalc 383 383
util 317 9 326
verilog 278 41 319
parasitics 199 17 216
spice 126 9 135
sdf 115 10 125
graph 100 11 111
power 96 8 104
test 22 22
Total 5,854 305 6,159
image

C++ Tests (GTest)

  • Covers delay calculation, graph construction, liberty parsing, network traversal, parasitics reduction, power analysis, SDC constraints, timing search, utility functions, and Verilog I/O
  • Each module has dedicated test fixtures with realistic design data (liberty, Verilog, SPEF, SDC)

Tcl Tests

  • Integration-level tests exercising the full STA flow through Tcl commands
  • Each test includes a .ok golden reference file for output verification

Note: Upstream Merge

This branch includes the upstream STA rel_3.0 merge (PR #288), which introduces the Scene/Mode architecture replacing Corner, enum class migrations, warning format changes, and Tcl command renames. All tests have been adapted to these API changes.

Test plan

  • All 6,159 regression tests pass (ctest -j64)
  • C++ tests: 5,854 pass
  • Tcl tests: 305 pass

🤖 Generated with Claude Code

jhkim-pii and others added 27 commits February 13, 2026 19:19
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
…ving `R#_` prefixes, improve temporary file creation with `mkstemp`, etc

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
- Remove stale line-number coverage comments (# Targets: line NNN, hit=0)
- Remove useless file-existence checks from verilog/sdf tests
- Delete 21 orphaned dcalc Tcl tests (C++ tests already cover them)
- Rename liberty_ccsn_ecsm -> liberty_ccsn (no ECSM libs available)
- Fix liberty_sky130_corners to use define_corners/-corner for real multi-corner testing
- Add report_checks per wireload model in liberty_wireload
- Fix test/regression to work from test/ directory (label mismatch)
- Refactor all module CMakeLists.txt with sta_module_tests() macro

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unnecessary catch blocks from Tcl test files across all modules,
add report_checks after each set_wire_load_model in liberty_wireload,
rewrite liberty_sky130_corners for actual multi-corner timing analysis
with define_corners, and expand C++ tests (TestSearchIncremental 8→36,
TestPower 71→96, TestSpice 98→126 tests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unnecessary catch blocks across all test modules (graph,
liberty, network, parasitics, power, sdc, sdf, search, spice,
verilog), expand C++ tests (TestSearchIncremental 8→36 tests,
TestPower 71→96, TestSpice 98→126), add report_checks after each
set_wire_load_model in liberty_wireload.tcl, and rewrite
liberty_sky130_corners.tcl with actual multi-corner timing analysis.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unnecessary catch blocks in network, parasitics, sdc, spice,
and util test modules. Add report_checks after set_wire_load_model
in parasitics_wireload.tcl to verify timing changes per wireload.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace wrong/non-existent command calls with correct OpenSTA APIs:
- sta::pin_slack → get_property $pin slack_max_rise
- sta::slow_drivers_cmd → sta::slow_drivers
- set_latch_borrow_limit → set_max_time_borrow
- remove_data_check → unset_data_check
- remove_clock → delete_clock
- reset_path → unset_path_exceptions
- sta::report_path_end 3-arg → sta::report_path_end2
- sta::design_power "NULL" → sta::design_power [sta::cmd_corner]
- report_path $path → sta::report_path_cmd $path
- connect_pin 3-arg → connect_pin net inst/port
- set_power_activity positional → -input_ports flag
- sta::is_clock [get_ports] → sta::is_clock [sta::get_port_pin]
- get_property $inst lib_name → liberty_cell/liberty_library
- get_property $pin net_name → [$pin net] + get_full_name
- get_property $net is_power → $net is_power method
- Removed unnecessary catch around sta::write_liberty

23 catch blocks removed. Tests now execute real API calls instead
of silently failing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document why each catch block is needed across 48 test files,
covering liberty, search, sdc, spice, network, parasitics, util,
and verilog modules.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove bare catch blocks that silently swallowed errors instead of
properly testing them. Fix underlying issues revealed by catch removal
including wrong API calls ([$role name] on strings, invalid properties
like cell_leakage_power/is_register, nonexistent Tcl bindings) and
incorrect library names. Update golden .ok files to match new output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ice tests

Remove dead code blocks that reference non-exposed Tcl commands (e.g.,
sta::find_pi_pole_residue) and delete all write_gate_spice catch blocks
from spice tests since write_gate_spice_cmd SWIG binding is missing,
making all write_gate_spice calls always fail. Added bug report for the
missing binding. All 6531 tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…edback

Remove useless empty-body if-blocks that check file size/existence without
doing anything, replacing them with meaningful puts output where appropriate.
Split monolithic verilog test files into individual per-test files with
their own .ok golden files. Update .ok files to match actual output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Replace empty-body checks and trivial file-existence assertions with
actual content verification and state validation in C++ unit tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Strengthen C++ test assertions in TestSdc and TestSearch with actual
value checks. Stabilize verilog_multimodule_write by using clear_sta
for isolated roundtrips. Add report_checks to wireload model tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Negate conditions in empty if-body assertions so the FAIL message
is printed directly (e.g. `if { $x != 0 } { } else { puts FAIL }`
becomes `if { $x == 0 } { puts FAIL }`).  Remove dead if/else blocks
where both branches were empty, strip meaningless status-only puts
from Tcl tests and their .ok golden files, and wrap a long line in
search_report_gated_datacheck.tcl to stay within 80 columns.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Add meaningful verification to liberty ECSM, sky130 corners, writer,
and roundtrip tests. Expand verilog specify, escaped-write, remove-cells,
write, and writer tests with content checks, roundtrip validation, and
error guards. Update corresponding .ok golden files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
- Add diff_files_sorted to test/helpers.tcl for hash-order-independent
  SDC comparison (fixes non-deterministic write_sdc output ordering)
- Use diff_files_sorted in sdc_derate_disable_deep and
  sdc_port_delay_advanced tests
- Remove stale coverage percentages from test comments (Comment 1)
- Remove unnecessary catch blocks in search property tests (Comment 3)
- Strengthen load-only tests with actual data verification (Comment 8)
- Remove orphan .ok files for deleted monolithic tests (Comment 9)
- Add golden .sdcok/.libok/.vok/.sdfok files for SDC/liberty/verilog
  write-and-diff tests
- Add -B (clean rebuild) option to make_coverage_report.sh
- Replace (void) casts and EXPECT_TRUE(true) with real assertions in
  TestSdc.cc and TestVerilog.cc

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
- Split oversized test files to stay under 5,000 lines per file:
  TestSdc.cc → TestSdcClasses.cc, TestSdcStaInit.cc, TestSdcStaDesign.cc
  TestSearchStaDesign.cc → TestSearchStaDesign.cc, TestSearchStaDesignB.cc
  TestLibertyStaBasics.cc → TestLibertyStaBasics.cc, TestLibertyStaBasicsB.cc
  TestNetwork.cc → TestNetwork.cc, TestNetworkB.cc
- Replace ~200+ (void) casts with proper EXPECT_* assertions across all
  C++ test files (dcalc, liberty, network, sdc, search, power, spice, util)
- Remove ~55 SUCCEED() and EXPECT_TRUE(true) no-op assertions
- Fix 6 load-only Tcl tests by adding diff_files verification with
  22 new .sdcok golden reference files
- Delete 7 orphan .ok files with no matching .tcl tests
- Add how_to_write_good_tests.md and TODO6.md documenting test quality rules

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Major upstream refactoring: Corner→Scene, Mode architecture, warning
format change (Warning ID:), command renames, and many API signature
changes. Adapted all C++ test files and TCL test scripts/expected
output files to pass with the new API. 6159/6159 tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
@jhkim-pii
Copy link

jhkim-pii commented Feb 24, 2026

The PR title is misleading.
The actual title is Add new Tcl and C++ test cases (but I cannot edit it because the author of this PR is openroad-ci).

@maliberty maliberty changed the title Merge upstream STA rel_3.0 and adapt all tests to new API Add new Tcl and C++ test cases Feb 24, 2026
@maliberty
Copy link
Member

Looking at https://jenkins.openroad.tools/job/OpenSTA-Public/job/PR-294/1/pipeline-overview/?selected-node=55 I see

21:17:55  Error: regression, 11 can't read "-e": no such variable
21:17:55  % 

which seems like an error but the stage is marked as passed. I don't see that any tests ran.

@jhkim-pii
Copy link

jhkim-pii commented Feb 25, 2026

I don't understand how the error is raised.

By the way, in this PR, ctest base regression is newly established. So the CI should use ctest (can use multi-core) instead of regression (single core).
@vvbandeira Could you please help?

@CLAassistant
Copy link

CLAassistant commented Feb 25, 2026

CLA assistant check
All committers have signed the CLA.

@maliberty
Copy link
Member

Changing to ctest will be a big job and probably should be its own PR. This PR has two issues

  1. Dockerfile.ubuntu22.04 ends in ENTRYPOINT ["OpenSTA/build/sta"] so you are not in a bash shell
  2. When that is commented out I see (locally):
./test/regression
Test project /workspace/w4/tools/OpenROAD/src/sta/build
No tests were found!!!

so the runner doesn't work

@dsengupta0628
Copy link
Contributor

dsengupta0628 commented Feb 26, 2026

I think this PR has one too many changes and it should be split to first checkin the new infrastructure? Just the CMakeLists and the folder structure with maybe 1 tests in each? Let @vvbandeira setup the new regression running methodology as part of OpenSTA's CI, and simply add the rest of the regressions?

I am also thinking if there is any way to consolidate so many regressions? Can we ask Gemini or whoever is your AI assistant, to combine some of them into single regressions- currently there are many thousands of them and very hard to review each, unfortunately. And going forward, whenever we make any development, we make sure to add regression to ensure the new code coverage.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
…unit tests

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
…CodeCoverage.sh like OpenROAD

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
… the upstream repo

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
@jhkim-pii
Copy link

@dsengupta0628 Thank you for your feedback.

[1]
I opened a new PR #300. Please review the test infra first.

[2]
How many test cases are good?
We can reduce the number of files or the number of test cases at the expense of some code coverage.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
@dsengupta0628
Copy link
Contributor

@dsengupta0628 Thank you for your feedback.

[1] I opened a new PR #300. Please review the test infra first.

[2] How many test cases are good? We can reduce the number of files or the number of test cases at the expense of some code coverage.

Thanks- I reviewed the test infra and had just one minor comment. Please see that PR

Regarding how many regressions are good: there isn't a correct answer to this- I would have thought a few 100 to begin with (~10 per module, C++ tests can be more), but you mentioned these run very fast so I suppose it is okay to keep them, and as we go forward, and hit something problematic, we can remove a regression or update it. I assume we will eventually start incorporating many consistency checks, and assertions in code too in our regression suite, so it may explode the testing time- but we will cross that bridge when we get there.

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.

5 participants