Refactor Tests

[n8mgr]

When writing tests it’s important to focus on testing the smallest, most isolated piece of functionality you can. This keeps tests focused and makes debugging future regressions easier.

Take the sqlite package as an example. The goal for the tests in this package should be to test how SQLite queries work and interact. Using TestScan as an example: it spins up a whole explorer node and brings in a chain manager, syncer, and all the other unrelated dependencies. Instead, aim to mock the specific database state needed for each test. It can be annoying and take more effort to set up, but it keeps your tests clean and focused. The goal is to test SQL interactions -- not the whole scan process.

If you want to test that scanning is working as expected, that should happen where the scanning code lives -- in the explorer package. The goal of tests isn’t just to say "this works," it's also to make breaking logic changes easier to detect. That way we've got multiple levels of validation that scanning is working as expected.

There are exceptions, but this is a good guidepost to follow.

Glancing through the code, I would recommend creating sub-issues and doing this piece by piece.

[chris124567]

Agree.

With regards to the SQLite/consensus tests, I tried to keep the v2 tests more "focused" in this regard but there's definitely improvements to be made across the board. But part of the reason we don't mock database state is because we are testing the functions that retrieve from the database but we also want to make sure that it was inserted properly. If we just have a test that inserts the data (i.e. transactions with file contracts) then we're merely testing whether the insert queries succeeded or not and not whether the queries themselves were correct or not.

[n8mgr]

Using your example of a transaction with file contract, I would structure one of the sqlite tests like this:

1. Manually build an apply update that includes a transaction and a file contract
2. Use getter to ensure the contract and transaction were both added as expected and return the expected state
3. Build an apply update that includes a transaction and a revision
4. Use the getter again to ensure the contract was updated and the transaction exists.

In this theoretical test, the goal isn't necessarily to only test that the insert query works (although I would recommend a separate test for that too), but to test the expected lifecycle and ensure database consistency. Most importantly, the focus is on verifying the state of the database and avoids dependencies like the chain manager or indexing loop which could unintentionally alter the state. Additional tests could expand on this further, such as including a storage proof, a revert update, etc.

Then in the explorer package, I would recommend a separate suite of tests to validate that contracts from the blockchain are being indexed correctly. For example:

1. Build a transaction that includes a file contract
2. Mine the block
3. Use the explorer getter methods to ensure the explorer is returning the expected state
4. Build a transaction that includes a revision for the contract
5. Mine the block
6. Check the state again

These tests should focus more on the chain manager and indexing logic since the database code is theoretically already well tested.

You could eventually layer that up with the API too:

1. Start up a node and API
2. Mine a block with a contract
3. Ensure the API client is returning the expected state

This adds duplication and overlap, but duplication in test coverage isn't inherently bad if it's testing different levels of a full system.

[ChrisSchinnerl]

Closing this for now since we merged a series of PRs in that regard.