Skip to content

test: Add test for account objects - #7625

Open
kuznetsss wants to merge 6 commits into
XRPLF:developfrom
kuznetsss:Add_test_for_account_objects
Open

test: Add test for account objects#7625
kuznetsss wants to merge 6 commits into
XRPLF:developfrom
kuznetsss:Add_test_for_account_objects

Conversation

@kuznetsss

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Add a test case for account objects checking it doesn't show cancelled offers.

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 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (6aed3bb) to head (60341d3).
⚠️ Report is 28 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7625     +/-   ##
=========================================
- Coverage     82.3%   82.3%   -0.0%     
=========================================
  Files         1024    1024             
  Lines        78459   78459             
  Branches      8975    8975             
=========================================
- Hits         64569   64558     -11     
- Misses       13881   13892     +11     
  Partials         9       9             

see 10 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.

seqs.reserve(numEntries);
for ([[maybe_unused]] auto _ : std::ranges::iota_view{0, numEntries})
{
json::Value params;

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.

can we add calls to testcase() to describe what each section is testing which rpc command?

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.

Could you please explain what you mean? The whole test is one test case, we just do a lot of rpc calls.

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.

Ah okay, I thought you were using different rpc commands but it seems that those for loops are for setting things up? If that's the case, then I think it's worth adding some comments.
Another thing is that we're using the submit command directly, but there's already tools like offer(Account, Amount, Amount)) which you can find them in Offer_test.cpp

Comment thread src/test/rpc/AccountObjects_test.cpp Outdated
void
testAccountObjectDoesntShowCancelledOffers()
{
testcase("AccountObjectDoesntShowCancelledOffers");

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.

This test case name doesn't seem to be very human-readable.

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.

Fixed

@kuznetsss
kuznetsss force-pushed the Add_test_for_account_objects branch from 2af88c9 to 6b88304 Compare July 1, 2026 13:57
@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

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.

2 participants