rcx: fix the diff_spef test helper - #11070
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a typo in src/rcx/test/rcx_aux.py by changing filename to filenamename in the write_spef function. The reviewer correctly identified that this introduces an undefined variable causing a NameError and suggested reverting this change and applying the fix to the correct function (diff_spef) instead.
| def write_spef(design, *, filename="", nets="", net_id=0, coordinates=False): | ||
| opts = rcx.SpefOptions() | ||
| opts.file = filename | ||
| opts.file = filenamename |
There was a problem hiding this comment.
This change introduces an undefined variable filenamename, which will cause a NameError when write_spef is called. Additionally, this modifies write_spef instead of diff_spef (located at line 113), which is the helper that actually has the undefined file variable. Please revert this change and apply the fix to diff_spef instead.
| opts.file = filenamename | |
| opts.file = filename |
| @@ -59,7 +55,7 @@ def extract_parasitics( | |||
|
|
|||
| def write_spef(design, *, filename="", nets="", net_id=0, coordinates=False): | |||
| opts = rcx.SpefOptions() | |||
| opts.file = filename | |||
| opts.file = filenamename | |||
|
I fixed the review issue. The accidental write_spef change is reverted, and only diff_spef now uses its filename argument. py_compile and an AST-based helper check both pass on the VM. I also updated the description so this PR claims only the helper fix, not the other diff_spef issues. |
|
I fixed the requested issue and added a small behavioral test for the helper. It calls |
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
0707cee to
e718f48
Compare
|
I corrected the description to cover only the |
Fixes the broken test helper reported in #11050.
The helper accepts a filename but was passing an undefined variable into DiffOptions. This patch changes only diff_spef to use the filename argument and leaves write_spef unchanged.
This PR covers the helper bug only. It does not claim to fix the separate diff_spef output and calibration issues from the report.
Tested on the VM:
bazel test //src/rcx/test:rcx_aux_test --test_output=errors