Found while writing the Roland S-7xx tests (D13). It is a gap in what the test suite proves, not a defect in shipped behaviour.
The problem
ADR-0005 exists because a filesystem does not always start at byte 0 — a Nero image puts 150 sectors of zeroed pregap in front of one, and getting it wrong reports an empty disc rather than an error. The natural way to test that is make_nrg() in tests/fixtures.py, which builds an image with a 150-sector pregap.
That test resolves at offset 0, not 307 200. make_nrg writes the DAOX track start at the end of the pregap, and NrgImage hands FlatImage exactly that range — so the container has already stripped the pregap before the origin probe ever runs. The probe finds the header at offset 0 of the cooked stream and the assertion passes for a reason unrelated to what it is named after.
The behaviour is correct. What is wrong is that a test named for the pregap case does not exercise it, so a regression in the origin probe would not be caught by it.
Why it matters beyond D13
Any backend whose ADR-0005 coverage rests on make_nrg has the same hole. Worth auditing tests/test_akai_fs.py, tests/test_emu3.py and tests/test_containers.py for assertions that look like they cover a non-zero origin and do not.
What D13 did about it
tests/test_roland_s7xx.py keeps the NRG test — it is still worth having, as it covers the container path — and adds test_origin_resolves_when_the_pregap_is_inside_the_cooked_stream, a flat image with the zeros genuinely in the stream, asserting origin.offset == 150 * 2048. The docstrings say which half each one covers.
Suggested fix
Either give make_nrg a mode that leaves the pregap inside the reported track range, or add the flat-image variant alongside the NRG one wherever a backend claims to test a non-zero origin. The second is what D13 did and is the smaller change.
Related
A second, smaller thing found in the same pass, filed here rather than separately since both are about tests proving less than they appear to:
Filename-keyed verification is unsound on macOS. A byte-identity sweep keyed on sanitised filenames reported 6 false mismatches on the Roland discs, all of them a case-insensitive filesystem resolving two disc names that differ only in case (BRS:RCA Tpt C_6E and C_6e) onto one file, so one is written as _2 and a name-keyed lookup reads the wrong WAV. Verifying by content gives 6392/6392. The 0.2.0 release notes already describe using multisets of hashes for exactly this reason; anything checking extraction should follow that and not go via paths.
Found while writing the Roland S-7xx tests (D13). It is a gap in what the test suite proves, not a defect in shipped behaviour.
The problem
ADR-0005 exists because a filesystem does not always start at byte 0 — a Nero image puts 150 sectors of zeroed pregap in front of one, and getting it wrong reports an empty disc rather than an error. The natural way to test that is
make_nrg()intests/fixtures.py, which builds an image with a 150-sector pregap.That test resolves at offset 0, not 307 200.
make_nrgwrites the DAOX track start at the end of the pregap, andNrgImagehandsFlatImageexactly that range — so the container has already stripped the pregap before the origin probe ever runs. The probe finds the header at offset 0 of the cooked stream and the assertion passes for a reason unrelated to what it is named after.The behaviour is correct. What is wrong is that a test named for the pregap case does not exercise it, so a regression in the origin probe would not be caught by it.
Why it matters beyond D13
Any backend whose ADR-0005 coverage rests on
make_nrghas the same hole. Worth auditingtests/test_akai_fs.py,tests/test_emu3.pyandtests/test_containers.pyfor assertions that look like they cover a non-zero origin and do not.What D13 did about it
tests/test_roland_s7xx.pykeeps the NRG test — it is still worth having, as it covers the container path — and addstest_origin_resolves_when_the_pregap_is_inside_the_cooked_stream, a flat image with the zeros genuinely in the stream, assertingorigin.offset == 150 * 2048. The docstrings say which half each one covers.Suggested fix
Either give
make_nrga mode that leaves the pregap inside the reported track range, or add the flat-image variant alongside the NRG one wherever a backend claims to test a non-zero origin. The second is what D13 did and is the smaller change.Related
A second, smaller thing found in the same pass, filed here rather than separately since both are about tests proving less than they appear to:
Filename-keyed verification is unsound on macOS. A byte-identity sweep keyed on sanitised filenames reported 6 false mismatches on the Roland discs, all of them a case-insensitive filesystem resolving two disc names that differ only in case (
BRS:RCA Tpt C_6EandC_6e) onto one file, so one is written as_2and a name-keyed lookup reads the wrong WAV. Verifying by content gives 6392/6392. The 0.2.0 release notes already describe using multisets of hashes for exactly this reason; anything checking extraction should follow that and not go via paths.