8000 test: enable TxQ unit tests work with variable reference fee by vvysokikh1 · Pull Request #5118 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

test: enable TxQ unit tests work with variable reference fee #5118

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

Merged
merged 3 commits into from
Mar 24, 2025

Conversation

vvysokikh1
Copy link
Collaborator
@vvysokikh1 vvysokikh1 commented Sep 6, 2024

High Level Overview of Change

Fix TxQ unit tests to be able to process reference fee value other than 10.

Context of Change

In preparation for potential reference fee change we would like to verify that fee change works as expected. The first step is to fix all unit tests to be able to work with different reference fee values.

Type of Change

  • [] Tests (you added tests for code that already exists, or your new feature included in this PR)

API Impact

None

Test Plan

Tested following reference fee values: 10, 20, 50, 100, 200, 500, 1000 by changing https://github.com/vvysokikh1/rippled/blob/2f432e812cb773048530ebfaf2e0e6def51e3cc2/src/test/jtx/impl/envconfig.cpp#L47

@vvysokikh1 vvysokikh1 changed the title test: enable TxQ unit tests work with variable reference fee test: enable TxQ unit tests work with variable reference fee (#5118) Sep 6, 2024
Copy link
codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.1%. Comparing base (53ea31c) to head (83ce629).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5118   +/-   ##
=======================================
  Coverage     78.1%   78.1%           
=======================================
  Files          790     790           
  Lines        67908   67908           
  Branches      8229    8226    -3     
=======================================
+ Hits         53024   53033    +9     
+ Misses       14884   14875    -9     

see 4 files with indirect coverage changes

Impacted file tree graph

@vvysokikh1 vvysokikh1 changed the title test: enable TxQ unit tests work with variable reference fee (#5118) test: enable TxQ unit tests work with variable reference fee Feb 21, 2025
@vvysokikh1 vvysokikh1 requested review from Bronek and ximinez February 28, 2025 13:48
@vvysokikh1 vvysokikh1 force-pushed the TxQ_tests_variable_reference_fee branch from 2a44022 to 83ce629 Compare March 3, 2025 14:32
@bthomee bthomee self-requested a review March 4, 2025 15:13
// Calculating expected median fee level based on known fee levels of median
// transaction levels.
auto
calcMedFeeLevel(FeeLevel64 const feeLevel1, FeeLevel64 const feeLevel2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't there an existing function you can call? Since the fee level is determined by the transactions seen and used in the non-test code, surely there must be a function that already does this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's the code that does that:

TxQ::FeeMetrics::scaleFeeLevel(Snapshot const& snapshot, OpenView const& view)

Unfortunately this is inside of a struct that is declared private inside of transaction queue. Without changes to the actual code it's not possible to use

@vvysokikh1 vvysokikh1 added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Mar 24, 2025
@bthomee bthomee merged commit 67028d6 into XRPLF:develop Mar 24, 2025
20 checks passed
Tapanito pushed a commit to Tapanito/rippled that referenced this pull request Mar 27, 2025
)

In preparation for a potential reference fee change we would like to verify that fee change works as expected. The first step is to fix all unit tests to be able to work with different reference fee values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0