fix: records_from_file raises IndexError when magic number is unrecognised (closes #75) - #81
Open
seam0814 wants to merge 1 commit into
Open
Conversation
…nised (closes Semi-ATE#75) `extension_from_magic_number_in_file()` returns an empty list when the file's content does not match any of the recognised magic-number signatures. `records_from_file.__init__` then dereferenced `compression[0]` unconditionally: compression = extension_from_magic_number_in_file(FileName) if compression[0] == '.xz': ... so any STDF file produced by older ATE tooling that does not carry the '.stdf' magic header (or simply the wrong header) raised IndexError: list index out of range at the very first line of the constructor, before the file was ever opened. The reporter's minimal "print all records" demo crashed for exactly this reason. Lift the optional index out into a local: compression_ext = compression[0] if compression else None if compression_ext == '.xz': ... else: # Assume standard binary stdf file ... and route the empty-magic case through the existing "assume standard binary stdf" fall-through branch. No code reshuffling beyond that — behaviour on every previously-handled magic value is byte-identical. Regression test in `tests/test_utils.py` uses `unittest.mock.patch` to force `extension_from_magic_number_in_file` to return `[]` and then calls `records_from_file` on a real FAR-only STDF blob, asserting the FAR record round-trips without raising. Full suite: 32 passed, 4 skipped. Note on a latent secondary symptom: when the IndexError fires inside `__init__`, `self.fd` is never assigned, so the subsequent `__del__` raises a chained `AttributeError: 'records_from_file' object has no attribute 'fd'`. With the IndexError gone this path is no longer reachable from the reported workflow, but `__del__` could still be made defensive in a follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
Gentle nudge — small guard against |
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
`extension_from_magic_number_in_file()` returns an empty list when the
file's content does not match any recognised magic-number signature.
`records_from_file.init` then dereferenced `compression[0]`
unconditionally:
```python
compression = extension_from_magic_number_in_file(FileName)
if compression[0] == '.xz':
...
```
So any STDF file produced by older ATE tooling that does not carry the
`.stdf` magic header (or simply the wrong header) raised
```
IndexError: list index out of range
```
at the very first line of the constructor, before the file was ever
opened — exactly the symptom in #75.
Closes #75.
Change
```diff
```
`compression[0]` was used at four call sites (`.xz`, `.bz2`, `.gz`,
`.zip`); each is rewritten against `compression_ext`. The empty-magic
case now falls through to the existing "assume standard binary stdf"
branch, which is already responsible for non-compressed files. No
behavioural change on any previously-handled magic value.
Test
Added `test_records_from_file_handles_unrecognised_magic_number` in
`tests/test_utils.py`. It uses `unittest.mock.patch` to force
`extension_from_magic_number_in_file` to return `[]` and then calls
`records_from_file` on a real FAR-only STDF blob, asserting the FAR
record round-trips without raising.
Full suite: 32 passed, 4 skipped, 0 failed.
Note on a latent follow-up
When the original `IndexError` fires inside `init`, `self.fd`
is never assigned, so the subsequent `del` raises a chained
`AttributeError: 'records_from_file' object has no attribute 'fd'`.
With the `IndexError` gone this path is no longer reachable from the
reported workflow, but `del` could still be made defensive in a
follow-up (`if getattr(self, 'fd', None) is not None: self.fd.close()`).
Happy to send that as a separate PR if you'd like.
Scope
Two-file change (`Semi_ATE/STDF/utils.py` + `tests/test_utils.py`),
no public API change, no behavioural change on any path that
previously worked.