Skip to content

[Repo Assist] Add JSON SaveOptions property tests + STJ benchmark prototype (#1671)#1694

Merged
dsyme merged 2 commits intomainfrom
repo-assist/improve-json-tests-and-stj-bench-20260313-f29e1e3425191501
Mar 13, 2026
Merged

[Repo Assist] Add JSON SaveOptions property tests + STJ benchmark prototype (#1671)#1694
dsyme merged 2 commits intomainfrom
repo-assist/improve-json-tests-and-stj-bench-20260313-f29e1e3425191501

Conversation

@github-actions
Copy link
Contributor

🤖 This is an automated pull request from Repo Assist, an AI assistant for this repository.

Task 9 — Testing Improvements: JSON property tests for all SaveOptions

Extends JsonParserProperties.fs with two new FsCheck property tests that were previously missing:

  • Parsing JsonValue formatted with None (indented) returns the same JsonValue — 500 random JsonValue trees serialized with JsonSaveOptions.None (pretty-printed/indented) and parsed back; verifies the formatted output is valid JSON that round-trips correctly.
  • Parsing JsonValue formatted with CompactSpaceAfterComma returns the same JsonValue — same check for JsonSaveOptions.CompactSpaceAfterComma (introduced in A new FSharp.Data.JsonSaveOptions value needed #1482).

The existing DisableFormatting property test (1000 cases) is unchanged. All three save-option variants are now covered by property tests.

Task 10 — Take the Repository Forward: STJ benchmark prototype for #1671

Adds tests/FSharp.Data.Benchmarks/JsonStjBenchmarks.fs — a working prototype of "Option 2" from the #1671 discussion (keep the JsonValue public API, use System.Text.Json as the parsing kernel).

The StjConverter module demonstrates the conversion path:

string → JsonDocument.Parse → walk JsonElement → JsonValue

JsonStjBenchmarks benchmarks this against the current hand-written parser on three real-world JSON files (GitHub issues, Twitter sample, WorldBank). Running dotnet run -c Release -- stj will give a direct comparison of:

  • Parse throughput (ops/sec)
  • Allocations (bytes/op)

This gives concrete data for the #1671 discussion without requiring any source-code changes to the main library. The StjConverter prototype lives entirely in the benchmarks project.

No new NuGet dependenciesSystem.Text.Json is part of the net8.0 shared framework (the benchmarks project already targets net8.0).

Also updates Program.fs to accept stj as a command-line argument and includes JsonStjBenchmarks in the default "run all" path.

Test Status

  • ✅ Build: 0 errors, 6 pre-existing warnings (NETSDK1212 trim warnings, pre-existing FS1182)
  • ✅ All 7 JsonParserProperties tests pass (including the 2 new property tests)
  • ✅ Benchmarks project builds cleanly

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

Task 9: Add two new FsCheck property tests to JsonParserProperties.fs verifying
that JsonValue roundtrips correctly through all three JsonSaveOptions variants
(None/indented and CompactSpaceAfterComma were previously untested; only
DisableFormatting was covered by the existing property test).

Task 10: Add JsonStjBenchmarks.fs — a concrete prototype of 'Option 2' from
#1671 (keep the JsonValue public API, use System.Text.Json as the parsing kernel).
The StjConverter module converts JsonDocument → JsonValue and benchmarks it
against the current hand-written parser on three real-world JSON files (GitHub,
Twitter, WorldBank). No new NuGet dependencies — System.Text.Json is part of the
net8.0 shared framework. Updates Program.fs to support 'stj' benchmark target.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dsyme dsyme marked this pull request as ready for review March 13, 2026 23:42
@dsyme dsyme merged commit cf28665 into main Mar 13, 2026
2 checks passed
@dsyme dsyme deleted the repo-assist/improve-json-tests-and-stj-bench-20260313-f29e1e3425191501 branch March 13, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant