Add unit tests for `tecDIR_FULL`

[Bronek]

Summary

We should add unit tests for tecDIR_FULL error wherever possible. Currently lines such as return tecDIR_FULL; are adorned with // LCOV_EXCL_LINE, but this error is now testable.

Since #5935 we have the means to move the last page in a directory to any arbitrary index (only on open ledger, but that should be sufficient for the purpose of writing unit tests).

Motivation

Improve unit test coverage

[BajanWave]

Hi @Bronek I am open to working on this. I just need to do some research on how tests are written in this repo.

[BajanWave]

Hi @Bronek , so I started looking into how to solve this issue. If I understand correctly I would find the occurrences of return tecDIR_FULL; in the repo and update the unit tests to cover those lines of code. As seen here. Let me know if there is anything else I am overlooking.

[Bronek]

@BajanWave thank you for volunteering to increase our unit test coverage; please see the change I made in Credentials_test.cpp in https://github.com/XRPLF/rippled/pull/5935/commits/759415afa64812c00fb2a8dd1e8e7e134fd79fd7#diff-d7897e33a8fc2faf3303c3179605aaf75e0385a5275a3fd0fa420a67c7c7072c as an example of a unit testa I'd like added for other types of objects / transaction types.

Please note we require signed commits before we can merge any PR, and also read both BUILD.md and CONTRIBUTING.md

[BajanWave]

Hi @Bronek,

I am running into an issue with one of the tests when I add the following line:

mptAlice.authorize({.account = bob, .err = tecDIR_FULL});

When the test suite runs without suppressing the intended tecDIR_FULL, it seems to trigger two failures. The first one is expected as part of the setup and is suppressed by .err=tecDIR_FULL ensuring the proper test ran, but the second one is unclear. Instead of the usual diagnostic output, the test runner only reports:

#295 failed

I am not sure what the root cause of that second failure is because nothing gets logged.

Are there any helper functions or documentation that can improve visibility or debugging for tests in this area? If not, do you have any recommendations on how to track down this kind of silent failure?

Thanks for any suggestions!

[ximinez]

#295 failed

This is, unfortunately, an artifact of what is probably a very old test. In short, every time expect is called, it increments a counter. The counter is reset with things like a new testcase. The original intention was that if a test failed, the dev would be able to could the expects and find what failed. Obviously not very scalable when you've got tests with loops and branches and just hundreds of checks.

So "#295 failed" means the 295th check failed. My suggestion is to run rippled in a debugger and put a breakpoint at runner.h line 263 (in the fail(std::string const& reason) function). If that doesn't help, please copy and paste more of the output from the failing test suite, and I or someone else might be able to help you narrow it down.

[BajanWave]

I see. Thanks for the tip. I think a nested expect inside a helper function without any related output is probably causing the problem. I will try running rippled with a debugger so I can catch exactly which expect is failing.

Also, the output only showed “#295 failed”, which made it hard to tell where it actually failed. I think looking directly at the runner function should help me figure out which part is triggering the failure.