feat(load2): add contract tests #4071
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates contract tests from the original load framework to load 2.0, replacing the simple ZeroTransferTest with a comprehensive RandomTest that can execute multiple test types with configurable weights.
- Introduces a new
RandomTeststruct that uses weighted sampling to select from multiple test types - Adds 10 new contract test implementations (ReadTest, WriteTest, StateModificationTest, etc.)
- Updates the main load generator to deploy a contract and use the new random test selection
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/load2/tests.go | Adds RandomTest implementation and 10 contract test types with shared utility functions |
| tests/load2/main/main.go | Updates main function to deploy contract and configure RandomTest with equal weights |
| ) { | ||
| require := require.New(tc) | ||
|
|
||
| index, ok := r.weighted.Sample(rand.Uint64N(r.totalWeight)) //#nosec G404 |
There was a problem hiding this comment.
How does this reflect load testing reproducibility, should we consider using a seeded random source?
There was a problem hiding this comment.
I'd second that suggestion. It wouldn't have to be the default behavior, but I think it would make sense to accept an optional seed.
tests/load2/tests.go
Outdated
| ) | ||
|
|
||
| var _ Test = (*ZeroTransferTest)(nil) | ||
| var ( | ||
| maxFeeCap = big.NewInt(300000000000) |
There was a problem hiding this comment.
Should we have a config structure for example
type LoadTestConfig struct {
MaxFeeCap *big.Int
DefaultTimeout time.Duration
GasTipMultiplier int64
}
tests/load2/main/main.go
Outdated
| func createRandomTest(contract *contracts.EVMLoadSimulator) (load2.RandomTest, error) { | ||
| // test params | ||
| count := big.NewInt(5) | ||
| weight := uint64(100) |
There was a problem hiding this comment.
Why is weight supplied if doesn't vary?
tests/load2/main/main.go
Outdated
|
|
||
| func createRandomTest(contract *contracts.EVMLoadSimulator) (load2.RandomTest, error) { | ||
| // test params | ||
| count := big.NewInt(5) |
There was a problem hiding this comment.
I would second copilot's comment - what does this value represent and why should it be 5?
There was a problem hiding this comment.
count indicates how many times to repeat an action if the test permits it - e.g.
The decision to set count = 5 is arbitrary but constant - load 1.0 used random counts which is not ideal if we want the individual tests (ReadTest, WriteTest, etc.) to be reproducible.
There was a problem hiding this comment.
Maybe summarize that in a comment?
| @@ -107,12 +107,15 @@ func main() { | |||
| chainID, err := workers[0].Client.ChainID(ctx) | |||
| require.NoError(err) | |||
|
|
|||
| randomTest, err := load2.NewRandomTests(ctx, chainID, &workers[0]) | |||
There was a problem hiding this comment.
Why randomTest but NewRandomTests?
| ) { | ||
| require := require.New(tc) | ||
|
|
||
| index, ok := r.weighted.Sample(rand.Uint64N(r.totalWeight)) //#nosec G404 |
There was a problem hiding this comment.
I'd second that suggestion. It wouldn't have to be the default behavior, but I think it would make sense to accept an optional seed.
| // | ||
| // This function handles the setup of the tests and also assigns each test | ||
| // an equal weight, making them equally likely to be selected during random test execution. | ||
| func NewRandomTests(ctx context.Context, chainID *big.Int, worker *Worker) (RandomWeightedTest, error) { |
There was a problem hiding this comment.
Why is this Tests plural despite returning RandomWeightedTest? Or maybe, why isn't it RandomWeightedTests given that it is a wrapper around a set of tests?
Why this should be merged
This PR migrates over the existing contract tests from the original load framework into load 2.0. This PR also replaces the type of test used in load 2.0 testing (
ZeroTransferTest=>RandomTest).How this works
Migration with the following caveats:
How this was tested
CI
Need to be documented in RELEASES.md?
N/A