Skip to content

docx_io split: scan/hardening/order-dependency characterization tests (prereq for slices 4-5) #71

Description

@LegalMarc

Design doc (read in full first): docs/design/docx_io_package_split.md — §3.2 items 2–5.

This issue lands the additional characterization tests the design doc requires before Slice 4 (scan.py) and Slice 5 (hardening.py/revisions.py) specifically — the least-tested areas of docx_io.py today. Tests only; no code moves here. It is separate from the golden-file harness (#70) because that harness gates all slices, whereas these deeper tests gate only slices 4 and 5.

Scope (each item is a §3.2 requirement)

  1. harden_document() direct unit tests (currently absent — docx_io.py:1711): RSID attribute removal; OLE/ActiveX w:object/w:control → placeholder-text replacement (both when the object is the run's only child and when siblings exist); and scrub_all_images=True drawing removal. Each asserted against parsed output XML, not "it didn't crash".
  2. Scanning/indexing unit tests isolating _build() / the _scan_* family from apply_replacements(): assert .text and .index content directly for each container type — especially text boxes via _scan_drawing_tag and nested tables via _scan_table_xml's recursion. These are the paths most likely to be silently altered by a refactor that moves iteration helpers between classes.
  3. Comment-visibility-map unit tests for _comment_visibility_map() (docx_io.py:1049): direct cases for each of the three states (hidden / deleted / visible via _is_hidden_run, w:del/w:moveFrom ancestry) plus the "comment start with no matching id in comments.xml" edge case.
  4. Order-dependency regression test pinning the implicit contract that save() only runs ZIP-level hardening (_rewrite_docx_zip, docx_io.py:382) when scrub_metadata() ran first (i.e. self._metadata_settings was set): one test calling scrub_metadata() then save() asserting hardening ran, and a companion test calling save() without a prior scrub_metadata() asserting hardening was skipped.

Non-negotiable requirements

  • All assertions against parsed/canonicalized XML, not raw bytes and not "no exception raised".
  • These tests must be green against the current unsplit docx_io.py — they characterize existing behavior, they do not change it. Do not modify docx_io.py.
  • The order-dependency test (item 5) is the single most important guard for Slice 5 and must fail loudly if the _metadata_settingssave() contract is ever broken.

Acceptance criteria

  • harden_document() unit tests: RSID removal, OLE/ActiveX placeholder replacement (only-child + with-siblings), scrub_all_images — all against parsed XML.
  • _build()/_scan_* unit tests asserting .text/.index per container type incl. text boxes and nested tables.
  • _comment_visibility_map() tests for hidden/deleted/visible + the orphan-comment-id edge case.
  • Order-dependency pair (hardening runs iff scrub_metadata ran first).
  • All green against current docx_io.py with no production-code edits.

Verification gate (all PRs in this set)

  • PYTHONPATH=src/python python3 -m pytest -q
  • swift build --package-path src/swift/MarcutApp
  • swift test --package-path src/swift/MarcutApp
  • bash scripts/release_preflight.sh

Dependencies

Independent of #70 (can land in parallel), but both this issue and #70 must be merged and green before #75 (scan.py) or #76 (hardening/revisions) may begin.


Land as its own PR; do not combine with other issues in this set. Do not reopen design-spike #25.

Metadata

Metadata

Assignees

No one assigned

    Labels

    afkAutonomous workflow-loop eligible

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions