Skip to content

Test/improve decoder coverage#48

Merged
alesanfra merged 4 commits intotoon-format:mainfrom
DiogoRibeiro7:test/improve-decoder-coverage
Mar 17, 2026
Merged

Test/improve decoder coverage#48
alesanfra merged 4 commits intotoon-format:mainfrom
DiogoRibeiro7:test/improve-decoder-coverage

Conversation

@DiogoRibeiro7
Copy link
Contributor

Linked Issue

Closes #N/A

Description

Made the CLI respect the --indent flag when decoding TOON so the emitted JSON formatting matches the user’s request, and added a regression test to lock in that behavior.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test coverage improvement

Changes Made

  • Passed the requested indent value through to json.dumps in decode_toon_to_json.
  • Added a CLI test that decodes TOON with --indent 4 and asserts the output uses four-space indentation.
  • Updated the CLI regression suite to cover the new formatting behavior.

SPEC Compliance

  • This PR implements/fixes spec compliance
  • Spec section(s) affected:
  • Spec version:

Testing

  • All existing tests pass
  • Added new tests for changes
  • Tests cover edge cases and spec compliance
    • uv run pytest tests/test_cli.py -k decode_indent

Pre-submission Checklist

  • My code follows the project's coding standards
  • I have run code formatting/linting tools
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated documentation if needed
  • I have reviewed the TOON specification for relevant sections

Breaking Changes

  • No breaking changes
  • Breaking changes (describe migration path below)

Additional Context

Ensures the CLI indent option behaves consistently during both encoding and decoding so users can control JSON formatting on both sides of the conversion.

@DiogoRibeiro7 DiogoRibeiro7 requested a review from a team as a code owner January 14, 2026 15:37
@alesanfra alesanfra assigned alesanfra and unassigned alesanfra Mar 14, 2026
Copy link

@alesanfra alesanfra left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for this contribution!

@DiogoRibeiro7
Copy link
Contributor Author

LGTM! Thanks for this contribution!

this will be merged? because i can't request reviews, the codeowners/maintaners should add automation for it.

Thanks in advance

@alesanfra
Copy link

At the moment we need 2 approvals to merge a PR, I'll ask @johannschopplich to add another approval.

@johannschopplich
Copy link
Contributor

Hey @DiogoRibeiro7 – thanks for the effort here, the CLI fix for --indent is clean and the test instinct is right. Two things:

The one-line fix in cli.py + 568 lines of new tests is a lot to review together. Would you mind splitting this into two PRs? The CLI fix is clearly mergeable on its own.

For the decoder tests: we actually have an official conformance test suite in toon-format/spec/tests/fixtures/ – language-agnostic JSON fixtures covering exactly what you're testing here (primitives, numbers, arrays, delimiters, error cases). The goal is to drive toon-python's test suite from those fixtures so all implementations stay consistent with each other and the spec.

Would you be open to rewriting the decoder tests to load and run the fixtures from the spec repo instead? Happy to point you to how this could work in practice.

@DiogoRibeiro7
Copy link
Contributor Author

Hey @DiogoRibeiro7 – thanks for the effort here, the CLI fix for --indent is clean and the test instinct is right. Two things:

The one-line fix in cli.py + 568 lines of new tests is a lot to review together. Would you mind splitting this into two PRs? The CLI fix is clearly mergeable on its own.

For the decoder tests: we actually have an official conformance test suite in toon-format/spec/tests/fixtures/ – language-agnostic JSON fixtures covering exactly what you're testing here (primitives, numbers, arrays, delimiters, error cases). The goal is to drive toon-python's test suite from those fixtures so all implementations stay consistent with each other and the spec.

Would you be open to rewriting the decoder tests to load and run the fixtures from the spec repo instead? Happy to point you to how this could work in practice.

@johannschopplich i will do that, thanks for the support and feedback

@DiogoRibeiro7
Copy link
Contributor Author

DiogoRibeiro7 commented Mar 17, 2026

Addressed review feedback:

  • Removed the large custom decoder edge-case test file (tests/test_decoder_edge_cases.py).
  • Kept this PR focused on the CLI
  • indent decode fix and its related CLI test update only.

@DiogoRibeiro7 DiogoRibeiro7 requested a review from alesanfra March 17, 2026 11:54
@alesanfra alesanfra merged commit 559dada into toon-format:main Mar 17, 2026
6 checks passed
@DiogoRibeiro7 DiogoRibeiro7 deleted the test/improve-decoder-coverage branch March 18, 2026 09:16
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.

3 participants