Skip to content

test(jsonc): make the JSONTestSuite harness compare parsed values - #7303

Open
tomas-zijdemans wants to merge 1 commit into
denoland:mainfrom
tomas-zijdemans:test-jsonc-harness-comparison
Open

test(jsonc): make the JSONTestSuite harness compare parsed values#7303
tomas-zijdemans wants to merge 1 commit into
denoland:mainfrom
tomas-zijdemans:test-jsonc-harness-comparison

Conversation

@tomas-zijdemans

Copy link
Copy Markdown
Contributor

The JSONTestSuite harness has never compared a parsed value. This makes it compare them.

getError(() => { JSON.parse(text); }) uses a block body with no return, so jsonResult and jsoncResult come back undefined every time. The assertEquals(jsonResult, jsoncResult) on line 47 has been comparing undefined to undefined since it was written. The test_transform/ corpus exists for exactly this check (number precision, duplicate keys, NFC/NFD keys, invalid codepoints) and none of it was running.

Switching to expression bodies returns the values.

Second fix: walk() had no extension filter, so README.md and test.ts were registered as JSON fixtures. Both parsers reject them, so they "passed". Added exts: [".json"], which is why the count drops from 408 to 406.

Does this catch a real bug? No. Everything still passes, so nothing was hiding. To confirm the assertion is live rather than merely present, I sabotaged the jsonc result and watched 159 tests fail, the 159 fixtures that parse successfully under both parsers. That same sabotage passed clean before this change.

No production code touched.

One follow-up I left out on purpose: the harness only checks whether something threw, never the type. That hides two fixtures, n_structure_100000_opening_arrays.json and n_structure_open_array_object.json, where jsonc throws RangeError and JSON.parse throws SyntaxError. Asserting the error type means first deciding what jsonc owes on recursion depth, so it gets its own issue rather than riding along here.

I used Claude Code to help investigate and write this change.

@github-actions github-actions Bot added the jsonc label Sep 3, 2026
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (ca58f94) to head (3afe9d6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7303   +/-   ##
=======================================
  Coverage   95.03%   95.04%           
=======================================
  Files         617      618    +1     
  Lines       51637    51640    +3     
  Branches     9359     9359           
=======================================
+ Hits        49075    49080    +5     
+ Misses       2021     2019    -2     
  Partials      541      541           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant