Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transaction deserializaation and enhance unit tests #593

Closed
wants to merge 13 commits into from

Conversation

sappenin
Copy link
Collaborator

@sappenin sappenin commented Feb 16, 2025

Once #594 is merged, this PR improves test coverage of all Transaction sub-classes by adding enhanced JSON deserialization coverage for each flavor of each Transaction. For example the Payment transaction can be deserialized via at least three different mechanisms:

  1. By pointing the Jackson ObjectMapper to a Transaction class (in which case, TransactionDeserializer will be engaged, delegating to the Immutable JSON implementation);
  2. By pointing the Jackson ObjectMapper to a Payment class (in which case, the TransactionDeserialer will be skipped and the Transaction interface will be used, delegating to the to the Immutable JSON implementation);
  3. By pointing the Jackson ObjectMapper to the Immutable implementation itself (e.g., an ImmutablePayment).

This PR adds this additional coverage to ensure proper JSON handling by future developers who may change certain aspects of each Transaction's Interface/Immutable or concrete deserializer without realizing that these changes might subtly affect deserialization based on other related classes.

@sappenin sappenin self-assigned this Feb 16, 2025
Copy link

codecov bot commented Feb 16, 2025

Codecov Report

Attention: Patch coverage is 28.87701% with 133 lines in your changes missing coverage. Please review.

Project coverage is 89.80%. Comparing base (a69b5f1) to head (7f7e86f).

Files with missing lines Patch % Lines
.../xrpl/xrpl4j/model/transactions/AccountDelete.java 25.00% 0 Missing and 3 partials ⚠️
...org/xrpl/xrpl4j/model/transactions/AccountSet.java 25.00% 0 Missing and 3 partials ⚠️
...ava/org/xrpl/xrpl4j/model/transactions/AmmBid.java 25.00% 0 Missing and 3 partials ⚠️
.../org/xrpl/xrpl4j/model/transactions/AmmCreate.java 25.00% 0 Missing and 3 partials ⚠️
.../org/xrpl/xrpl4j/model/transactions/AmmDelete.java 25.00% 0 Missing and 3 partials ⚠️
...org/xrpl/xrpl4j/model/transactions/AmmDeposit.java 25.00% 0 Missing and 3 partials ⚠️
...va/org/xrpl/xrpl4j/model/transactions/AmmVote.java 25.00% 0 Missing and 3 partials ⚠️
...rg/xrpl/xrpl4j/model/transactions/AmmWithdraw.java 25.00% 0 Missing and 3 partials ⚠️
...rg/xrpl/xrpl4j/model/transactions/CheckCancel.java 25.00% 0 Missing and 3 partials ⚠️
.../org/xrpl/xrpl4j/model/transactions/CheckCash.java 25.00% 0 Missing and 3 partials ⚠️
... and 35 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #593      +/-   ##
============================================
- Coverage     92.01%   89.80%   -2.22%     
- Complexity     1848     1893      +45     
============================================
  Files           383      383              
  Lines          5125     5305     +180     
  Branches        435      524      +89     
============================================
+ Hits           4716     4764      +48     
  Misses          273      273              
- Partials        136      268     +132     

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

@sappenin sappenin closed this Feb 16, 2025
@sappenin sappenin deleted the df/fixes-590-unl-modify branch February 16, 2025 22:32
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.

1 participant