Skip to content

fix(coverage): handle malformed JSON root and propagate tool exit failure - #529

Merged
0xsamuel1 merged 3 commits into
soroban-testkit:mainfrom
promisetechhub1788-cyber:fix/coverage-json-malformed-and-exit-failure
Oct 20, 2026
Merged

0xsamuel1 merged 3 commits into
soroban-testkit:mainfrom
promisetechhub1788-cyber:fix/coverage-json-malformed-and-exit-failure

Conversation

@promisetechhub1788-cyber

@promisetechhub1788-cyber promisetechhub1788-cyber commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

[coverage]` handle JSON output that is empty or malformed

augment_coverage_json() used if let Some(obj) = root.as_object_mut() which silently skipped injecting package_summaries/file_summaries/summary when the JSON root was not an object (e.g. null, [], "string", 42). The function returned Ok with an empty CoverageSummary and an unaugmented output file, giving no signal that the output was unusable.

Fix: replaced the silent if let with .ok_or_else(|| CliError(...)) so any non-object root immediately returns an actionable error describing the actual JSON type received.

[coverage]` avoid claiming success when the coverage tool exits unsuccessfully

run() already checked status.success() and returned early with a CliError on non-zero exit. However, there were no focused tests verifying this contract. Added unit and integration tests that assert the CLI always exits non-zero and emits actionable (never success-claiming) stderr for every reachable failure path.

Changes

crates/cli/src/commands/coverage.rs

  • Fix: augment_coverage_json — replace if let Some(obj) = root.as_object_mut() with .ok_or_else(|| CliError(...)) to surface unexpected root types as errors
  • Unit tests added:
    • augment_coverage_json_rejects_non_object_root
    • augment_coverage_json_rejects_malformed_json
    • validate_coverage_outputs_fails_for_malformed_json_content
    • coverage_failure_message_never_claims_success

crates/cli/tests/coverage.rs

  • Integration tests added:
    • json_output_flag_rejected_before_tool_is_spawned
    • cli_never_claims_success_on_argument_validation_failure
    • tool_exit_failure_is_propagated_not_swallowed
    • json_format_arg_validation_guards_augmentation_path

Verification

cargo fmt --check
cargo test -p soroban-testkit-cli
cargo test --workspace --doc

closes #453
closes #452
closes #455
closes #454

…lure

Two regression fixes for the coverage command:

1. [Bug] handle JSON output that is empty or malformed
   augment_coverage_json() previously used
   which silently skipped injecting package_summaries/file_summaries/summary when
   the JSON root was not an object (null, array, string, number). This meant the
   command returned Ok with an empty CoverageSummary and an unaugmented output
   file, giving no signal to the caller that the output was unusable.

   Fixed by replacing the silent if-let with .ok_or_else(|| CliError(...))? so
   any non-object root immediately returns an actionable error describing the
   actual JSON type received.

2. [Bug] avoid claiming success when the coverage tool exits unsuccessfully
   The run() function already checked status.success() and returned early with a
   CliError on non-zero exit, but there were no focused tests verifying this
   contract. Added unit and integration tests that assert the CLI exits non-zero
   and emits diagnostic (never success-claiming) output for every reachable
   failure path.

Unit tests added (coverage.rs mod tests):
- augment_coverage_json_rejects_non_object_root (array, null, string, number roots)
- augment_coverage_json_rejects_malformed_json (syntax errors, truncated JSON)
- validate_coverage_outputs_fails_for_malformed_json_content
- coverage_failure_message_never_claims_success

Integration tests added (crates/cli/tests/coverage.rs):
- json_output_flag_rejected_before_tool_is_spawned
- cli_never_claims_success_on_argument_validation_failure
- tool_exit_failure_is_propagated_not_swallowed
- json_format_arg_validation_guards_augmentation_path

Affected paths: crates/cli/src/commands/coverage.rs
                crates/cli/tests/coverage.rs
@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@promisetechhub1788-cyber Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@0xsamuel1
0xsamuel1 merged commit c58d2a5 into soroban-testkit:main Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants