systest: Introduce AcquireSystem

[ajsutton]

Description

Support having a system returned rather than having to supply a callback. To avoid adjusting all tests at once (and potentially causing a lot of conflicts), the callback approach is still supported. Will follow up with converting tests if we're happy with this change.

Tests

Updated systest_test.go but it appears to just be testing the mock.
Converted isthmus/fees_test.go to use the acquire approach.

[codecov]

Codecov Report

Attention: Patch coverage is 19.04762% with 17 lines in your changes missing coverage. Please review.

Project coverage is 42.45%. Comparing base (4165218) to head (2f64b74).
Report is 73 commits behind head on develop.

Files with missing lines	Patch %	Lines
devnet-sdk/testing/systest/systest.go	19.04%	17 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #15039      +/-   ##
===========================================
- Coverage    46.36%   42.45%   -3.91%     
===========================================
  Files         1227     1055     -172     
  Lines       103054    92819   -10235     
===========================================
- Hits         47779    39406    -8373     
+ Misses       51910    50216    -1694     
+ Partials      3365     3197     -168     

Flag	Coverage Δ
cannon-go-tests-32	?
cannon-go-tests-64	?
contracts-bedrock-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
devnet-sdk/testing/systest/systest.go	74.25% <19.04%> (-6.39%)	⬇️

... and 179 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[janjakubnanista]

This is where I would be a bit hesitant with this approach - when does this cancel run exactly?

And what are the implications for when tests are run in parallel (I am new to go so please feel free to discard my comments) - do we get a new t instance for every test, with t.Cleanup being only called once per each? What happens if we acquire two systems, does one Cleanup override the other one?

The advantage of the original approach here was that the lifecycle was limited to this function, if you expose & return the system, there are some more details to consider.

[ajsutton]

t.Cleanup is the canonical way to clean up test resources in go tests. You do get a new t for each test instance and multiple calls to Cleanup will all get run at the end of the test so it works fine if you acquire multiple systems still. We should be leveraging the testing tools that go provides like this rather than trying to build it all ourselves, especially given the callback approach adds a bunch of extra boilerplate and complexity to the test code.

Basically using defer we avoid the need to understand t.Cleanup in this particular case (but it's used in so many other places understanding it is really table stakes), but every single test has to write multiple functions and understand this callback flow because the code no longer executes linearly.

[opgitgovernance]

This pr has been automatically marked as stale and will be closed in 5 days if no updates

[ajsutton]

Giving up on getting reviews for this.