test: enable unit tests to work with variable reference fee

[vvysokikh1]

High Level Overview of Change This PR is a continuation of work done in: #5118 Fix remaining 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 [x ] 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

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.1%. Comparing base (67028d6) to head (b9a5163). Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5145     +/-   ##
=========================================
- Coverage     78.1%   78.1%   -0.0%     
=========================================
  Files          790     790             
  Lines        67990   67989      -1     
  Branches      8253    8254      +1     
=========================================
- Hits         53092   53088      -4     
- Misses       14898   14901      +3     

see 4 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Bronek]

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

Can you pls add a command line option to set this value ? It will probably go somewhere in app/main/Main.cpp (look for sections guarded by #ifdef ENABLE_TESTS )

[vvysokikh1]

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

Can you pls add a command line option to set this value ? It will probably go somewhere in app/main/Main.cpp (look for sections guarded by #ifdef ENABLE_TESTS )

This is actually quite tricky to do in runtime. Almost all tests allocate the Env and Config independently and it would require significant effort to propagate the fee value from startup params.

Feasible option is to make it compile time setting in CMake

[Bronek]

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

Can you pls add a command line option to set this value ? It will probably go somewhere in app/main/Main.cpp (look for sections guarded by #ifdef ENABLE_TESTS )

This is actually quite tricky to do in runtime. Almost all tests allocate the Env and Config independently and it would require significant effort to propagate the fee value from startup params.

Feasible option is to make it compile time setting in CMake

Yeah in that case just pls add an option to CMake (files RippledSettings.cmake and RippledCore.cmake); it is important that we can run unit tests with an arbitrary fee, so the new features do not revert to the hardcoded default 10.

[vvysokikh1]

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

Can you pls add a command line option to set this value ? It will probably go somewhere in app/main/Main.cpp (look for sections guarded by #ifdef ENABLE_TESTS )

This is actually quite tricky to do in runtime. Almost all tests allocate the Env and Config independently and it would require significant effort to propagate the fee value from startup params. Feasible option is to make it compile time setting in CMake

Yeah in that case just pls add an option to CMake (files RippledSettings.cmake and RippledCore.cmake); it is important that we can run unit tests with an arbitrary fee, so the new features do not revert to the hardcoded default 10.

I've added a draft PR with this workflow added: #5159

[vvysokikh1]

Almost good to go. I'm wondering, in a possible follow-up PR, whether we can make it a lot harder to forget to close the test ledger by making it explicit. For instance, the various env function calls, such as fund, offer, and trust could take an extra parameter, bool close = true, which would then call env.close() unless the argument is explicitly set to false.

From the moment we enable different reference fee testing, it will be not easy to push the tests that break this since they would fail on CI. We could technically add some additional precautions to make ledger close after fund, but I'm not capable to answer how much efforts we would need to make it work. There's plenty of tests that don't close ledger at all so modifying all those would be grind job