Use nextest config for fine-tuning tests

[wileyj]

we ran into an issue resolved by this commit: 82c36d7

what was happening was that the test in the commit above was timing out on a remote runner, but passing locally.
likely due to this issue in their docs: https://nexte.st/docs/configuration/threads-required/?h=threads

for the above test, i was able to confirm that using that threads-required flag for this specific test allowed it to pass.

i think we should be able to utilize this config to more finely-tune how we run tests locally and on remote blackboxes to reduce transient failures.

https://nexte.st/docs/configuration/per-test-overrides/
https://nexte.st/docs/configuration/test-groups/

[orsissimo]

I've been working on this issue and have created a configuration in .config/nextest.toml that implements the threads-required flag for the test_try_make_response test in the postblock_proposal module.

My solution uses:

[[profile.default.overrides]]
filter = "test(net::api::tests::postblock_proposal::test_try_make_response)"
threads-required = 2

[[profile.ci.overrides]]
filter = "test(net::api::tests::postblock_proposal::test_try_make_response)"
threads-required = 2
slow-timeout = { period = "90s", terminate-after = 1 }

I've been trying to reproduce the timeout issue locally but haven't been able to, even after simulating resource constraints with Docker. The test passes in my environment both with and without the configuration.

@wileyj Could you share any additional details on how to reproduce the issue or confirm if this configuration approach would solve the problem on your remote runners?

I can then focus on making the configuration more generic.

[wileyj]

Going off of memory here, but I think the root cause was addressed in #5669.

the nextest config is still something to aspire to though, since there are a lot of different tests we're treating as essentially the same. prior to the PR, a nextest config for this would have specified that this test should run single-threaded i think (again, going off of memory, but i think the issue with it was that multiple threads were conflicting with each other).

we treat tests today as a binary choice more of less: if bitcoind is required, use a single thread. if bitcoind is not required - use all available cpu (threads). in most cases, it works out fine - but occasionally we run into issues like here. i think a nextest.toml could be a good solution to managing how tests run.

i will add that reproducing it was hard - i was unable to reproduce on a local machine, it only ever manifested in a github runner where the logs were not very helpful. i wish i had more info to share here - it's a been a while and i just don't recall how we identified the issue as threading.

[orsissimo]

@wileyj Thanks for the information.

I attempted to reproduce the issue with a custom GitHub action and even tried testing in a VM with limited resources, but I wasn't able to trigger the failure of the test. This aligns with your experience that the issue was difficult to reproduce outside of specific GitHub runner environments.

I've been exploring a Nextest configuration that could help prevent similar issues in the future. Based on your comment about treating tests as a binary choice (single thread for bitcoind tests, all CPUs for others), I've set up a basic configuration that addresses the specific test mentioned and provides a framework for better test management:

# .config/nextest.toml
[profile.default]
fail-fast = false
retries = 0

# The specific test that had issues
[[profile.default.overrides]]
filter = 'test(net::api::tests::postblock_proposal::test_try_make_response)'
threads-required = 1

# Overrides for bitcoind tests (we need to properly filter test names)
[[profile.default.overrides]]
filter = 'test(test_pox_|bitcoin_)'
threads-required = 1

What would be the most reliable way to identify all the bitcoind-dependent tests in the codebase? I imagine that tests are those that have the BITCOIND_TEST variable = 1 in them, but if there is no consistent naming pattern, do you think it would be good to add a special keyword or rename the tests that require bitcoind to make them easier to filter in the Nextest configuration?