Skip to content

test: Add tests for TMProofPathResponse and TMReplayDeltaResponse invalid hash/key sizes#7593

Open
mathbunnyru wants to merge 6 commits into
XRPLF:developfrom
mathbunnyru:test_incorrect_key
Open

test: Add tests for TMProofPathResponse and TMReplayDeltaResponse invalid hash/key sizes#7593
mathbunnyru wants to merge 6 commits into
XRPLF:developfrom
mathbunnyru:test_incorrect_key

Conversation

@mathbunnyru

Copy link
Copy Markdown
Contributor

High Level Overview of Change

The change was implemented in 3.1.3, so let's add a test for it.

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.0%. Comparing base (0711a7b) to head (d7bc91f).
⚠️ Report is 5 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7593     +/-   ##
=========================================
- Coverage     82.0%   82.0%   -0.0%     
=========================================
  Files         1007    1007             
  Lines        76890   76891      +1     
  Branches      8976    8971      -5     
=========================================
  Hits         63049   63049             
- Misses       13832   13833      +1     
  Partials         9       9             

see 6 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mathbunnyru mathbunnyru added this to the 3.3.0 milestone Jun 23, 2026
godexsoft
godexsoft previously approved these changes Jun 26, 2026

@godexsoft godexsoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving but there is a question :rage2:

{
testcase("base_uint: general purpose tests");

#ifdef NDEBUG

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed?

@mathbunnyru mathbunnyru Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe not anymore, let's try to remove it

Update: yes, it's needed, we can't guarantee that the data will be 0 in Debug builds. I know it's not great, but this PR is a test only, so if it needs to be improved, it should be done separately.

@godexsoft godexsoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:rage1:

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.

2 participants