fix: TEST_NUM_from_record reads full 4-byte U*4 (closes #76) - #79
Open
seam0814 wants to merge 1 commit into
Open
fix: TEST_NUM_from_record reads full 4-byte U*4 (closes #76)#79seam0814 wants to merge 1 commit into
seam0814 wants to merge 1 commit into
Conversation
`TEST_NUM` is a U*4 field at byte offset 4..7 (inclusive) in PTR/MPR/FTR
records, so the slice into `struct.unpack` must be `record[4:8]`, not
`record[4:7]`. The 3-byte slice raised `struct.error: unpack requires a
buffer of 4 bytes` on every call, making the function effectively dead
on arrival.
Also:
- wrap the unpack result with `int(...)[0]` so the success path matches
the sentinel return value (`-1`, an int) and callers don't have to
branch on tuple vs int
- fix the docstring offset table (4:7 → 4:8)
- add three parametric pytest cases:
- round-trip with TEST_NUM = 0x12345678 (> 2**24, exposes any
MSB-byte truncation) under both little- and big-endian
- sentinel path returns int(-1), not tuple, for non-test records
- all three test record subtypes (PTR 15/10, MPR 15/15, FTR 15/20)
read TEST_NUM correctly
Full suite: 31 passed, 4 skipped.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
Gentle nudge — small parser fix reading full 4-byte U*4 for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TEST_NUM_from_recordextracts the test number from aPTR,MPRorFTRrecord.TEST_NUMis aU*4field at byte offset 4..7(inclusive), so the slice fed into
struct.unpack(\"...I\", ...)mustbe
record[4:8], notrecord[4:7].The 3-byte slice raises
struct.error: unpack requires a buffer of 4 byteson every invocation, so the function is effectively dead onarrival for the only record types it is supposed to handle.
Closes #76.
Change
Semi_ATE/STDF/utils.py—TEST_NUM_from_record:record[4:7]→record[4:8](the actual bug)int(...)[0]so the success pathreturns the same type as the sentinel (
-1, anint). Withoutthis, callers have to branch on
tuplevsint— almostcertainly unintended given the sentinel is already an int.
4:7→4:8)Test
Three new parametric pytest cases in
tests/test_utils.py:TEST_NUM = 0x12345678— value is larger than2**24, so any MSB-byte truncation would surface. Runs under bothlittle- and big-endian.
FAR) returnsint(-1), not a tuple.PTR (15,10),MPR (15,15), andFTR (15,20)all readTEST_NUMcorrectly.Full suite: 31 passed, 4 skipped, 0 failed.
```
$ python -m pytest tests/test_utils.py -v -k TEST_NUM
tests/test_utils.py::test_TEST_NUM_from_record_roundtrip[<] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_roundtrip[>] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_returns_int_for_non_test_record PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[10] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[15] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[20] PASSED
```
Scope
Pure bug fix, no public API change beyond the (almost certainly
unintended) tuple→int return-type tightening. No other call sites of
TEST_NUM_from_recordexist in the repository.