Skip to content

Restore JSON regression coverage for late non-ASCII text - #2601

Merged
Adam Fourney (afourney) merged 1 commit into
mainfrom
tests/restore-late-json-charset
Oct 4, 2026
Merged

Adam Fourney (afourney) merged 1 commit into
mainfrom
tests/restore-late-json-charset

Conversation

@afourney

Copy link
Copy Markdown
Member

The test consolidation removed coverage for JSON whose first non-ASCII character appears well into the file. Existing charset tests introduce Unicode near the beginning, allowing an incorrectly shortened detection sample to go unnoticed.

Add json_late_non_ascii.json, derived from the existing test.json, to the shared CLI and library vectors. Its first non-ASCII byte is at offset 10,779; a fixture check preserves that property. The vector verifies UTF-8 detection and preservation of the original fields and late Unicode text.

Validation:

  • 48 targeted JSON-vector and charset tests passed; four external-URL cases were skipped under the existing CI settings.
  • Temporarily limiting charset detection to 8 KiB in the test process caused five new vector checks to fail, confirming the restored coverage catches the regression.
  • git diff --check passed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The fixture and vector assertions correctly exercise the intended regression across existing test entry points.

Review effort: Balanced
Findings: None

What changed in this PR

Restores regression coverage for charset detection when Unicode appears late in JSON input.

Changes:

  • Adds a UTF-8 JSON fixture with a long ASCII prefix.
  • Adds shared CLI/library conversion checks.
  • Verifies the first non-ASCII byte falls between 8 KiB and 64 KiB.
File Description
packages/​markitdown/​tests/​test_module_misc.py Validates the fixture’s charset-sampling boundary.
packages/​markitdown/​tests/​test_files/​json_late_non_ascii.json Provides late-Unicode JSON input.
packages/​markitdown/​tests/​_test_vectors.py Adds shared conversion expectations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants