De-duplicate `needs_strict_codec` between `tests/conftest.py` and `tests/test_verify.py`

Open Beginner friendly
#608 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Quiet
Tech stack
python
Domain
testing-qa

Research direction

Compare tests/conftest.py:65 with the local definition and helper at tests/test_verify.py:56-59. Remove the duplicate and use the shared definition as the other affected test files do, preserving the CI-context comment with the shared definition. Run the affected tests with -rs and confirm they execute rather than become skipped.

Written by the indexing model from the issue text.

Description

enhancement P3

needs_strict_codec now exists twice in the test tree: the shared copy in tests/conftest.py and a local one in tests/test_verify.py.

State

The marker skips a test when the host codec can decode byte 0xff — every ISO-8859-x and cp125x codec maps all 256 byte values, so under such a locale a strict-decode test goes vacuously green rather than exercising the fault it is named for.

  • tests/conftest.py:65 — the shared copy, added by the #380/#383 bundle so phase 2's tests could import it. Used by tests/test_multiplexer.py, tests/test_hook_bus.py and tests/test_probe.py.
  • tests/test_verify.py:59 — a local copy predating it, with its own _codec_rejects_bad_byte helper. Used at :296, :1090 and :1114.

The two are byte-identical in condition and skip reason.

Why the duplication was left in place

test_verify.py is otherwise untouched by the #380/#383 fix. De-duplicating it would have dragged a large, unrelated file into a bundled fix whose whole discipline was to claim exactly what it fixed and no more. Deliberate, and named in that PR's "Out of scope" section — filed here so it does not simply rot.

Asked for

Delete the local copy and its _codec_rejects_bad_byte helper from tests/test_verify.py; import the shared one from conftest, as the other three files do.

Worth carrying over rather than dropping: the comment above the local copy (tests/test_verify.py:56-58) records that CI is always on the exercising side of this skip — the Linux legs run UTF-8 and the Windows legs set PYTHONUTF8=1 (.github/workflows/ci.yml). That fact belongs with the shared definition, which currently does not state it.

Verification

Mechanical, but confirm the tests still run rather than newly skipping — a marker refactor that silently flips three tests to skipped is indistinguishable from green. -rs on the affected file, or an explicit assertion on the collected outcome, is enough.

Dominant language
Python
Stars
137
Forks
60
Avg merge
1d 3h
Merged PRs (30d)
44

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from bmad-code-org/bmad-loop

All issues in bmad-code-org/bmad-loop

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.