Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CCIP Load Test connected to crib #15404

Closed
wants to merge 17 commits into from
Closed

CCIP Load Test connected to crib #15404

wants to merge 17 commits into from

Conversation

0xAustinWang
Copy link
Contributor

Copy link
Contributor

github-actions bot commented Nov 25, 2024

AER Report: CI Core

aer_workflow , commit , Clean Go Tidy & Generate , Detect Changes , Scheduled Run Frequency , GolangCI Lint (.) , GolangCI Lint (integration-tests/load) , GolangCI Lint (integration-tests) , GolangCI Lint (deployment) , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , test-scripts , lint , SonarQube Scan

1. Not enough arguments in call to env.GetConfig:[Golang Lint (deployment)]

Source of Error:
environment/crib/env_test.go:13:12: not enough arguments in call to env.GetConfig
	have ()
	want (map[uint64]string)
**Why**: The function `env.GetConfig` is being called without any arguments, but it requires a `map[uint64]string` as an argument.

Suggested fix: Pass the required map[uint64]string argument to the env.GetConfig function call.

2. Assignment mismatch: 1 variable but env.GetConfig returns 2 values:[Golang Lint (deployment)]

Source of Error:
environment/crib/env_test.go:13:12: assignment mismatch: 1 variable but env.GetConfig returns 2 values (typecheck)
**Why**: The function `env.GetConfig` returns two values, but only one variable is being used to capture the return values.

Suggested fix: Use two variables to capture both return values from env.GetConfig.

3. No go files to analyze:[Golang Lint (integration-tests/load)]

Source of Error:
level=error msg="Running error: context loading failed: no go files to analyze: running `go mod tidy` may solve the problem"
**Why**: The linter could not find any Go files to analyze in the specified directory.

Suggested fix: Ensure that there are Go files in the integration-tests/load directory or run go mod tidy to clean up the module dependencies.

4. Cannot use cd.wsRPCs as []devenv.CribRPCs value in struct literal:[Golang Lint (integration-tests)]

Source of Error:
testsetups/ccip/test_helpers.go:658:15: cannot use cd.wsRPCs (variable of type []string) as []devenv.CribRPCs value in struct literal
**Why**: The variable `cd.wsRPCs` of type `[]string` is being used where a `[]devenv.CribRPCs` type is expected.

Suggested fix: Convert cd.wsRPCs to the expected type []devenv.CribRPCs.

5. Cannot use cd.httpRPCs as []devenv.CribRPCs value in struct literal:[Golang Lint (integration-tests)]

Source of Error:
testsetups/ccip/test_helpers.go:659:15: cannot use cd.httpRPCs (variable of type []string) as []devenv.CribRPCs value in struct literal (typecheck)
**Why**: The variable `cd.httpRPCs` of type `[]string` is being used where a `[]devenv.CribRPCs` type is expected.

Suggested fix: Convert cd.httpRPCs to the expected type []devenv.CribRPCs.

6. Declared and not used: ownerChainB:[Golang Lint (integration-tests)]

Source of Error:
integration-tests/smoke/ccip/ccip_usdc_test.go:52:2: declared and not used: ownerChainB (typecheck)
**Why**: The variable `ownerChainB` is declared but not used in the code.

Suggested fix: Remove the declaration of ownerChainB if it is not needed, or use it appropriately in the code.

7. Updates to go.mod needed; to update it:[Clean Go Tidy & Generate]

Source of Error:
go: updates to go.mod needed; to update it:
	go mod tidy
error: exit status 1
**Why**: The `go.mod` file is not up-to-date and requires running `go mod tidy` to clean up and update the module dependencies.

Suggested fix: Run go mod tidy to update the go.mod file and clean up any unnecessary dependencies.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

deployment/environment.go Outdated Show resolved Hide resolved
integration-tests/load/ccip/helpers.go Outdated Show resolved Hide resolved
Comment on lines 51 to 57
"sourceSelector": fmt.Sprintf("%d", src),
"destinationSelector": fmt.Sprintf("%d", dst),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be easier to use names/chain id instead? Labelling by selector can create confusion as selector is a large number


func SendMetricsToLoki(l zerolog.Logger, lc *wasp.LokiClient, updatedLabels map[string]string, metrics *LokiMetric) {
if err := lc.HandleStruct(wasp.LabelsMapToModel(updatedLabels), time.Now(), metrics); err != nil {
l.Error().Err(err).Msg(ErrLokiPush)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you not want the test to fail if it fails to push?


return router.ClientEVM2AnyMessage{
Receiver: rcv,
Data: common.Hex2Bytes("hello world"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using some identifier here like msg with id to distinguish.

m.env.Chains[src].DeployerKey.Value = fee
defer func() { m.env.Chains[src].DeployerKey.Value = nil }()
}
_, err = r.CcipSend(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you approving router to spend fee tokens in the beginning?

// Parse all events from the simulated chains, send to Loki
// step 4: teardown
// Stop the chains, cleanup the environment
func TestCCIPLoad_RPS(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of code in this can be extracted to common functions. You might need more than one load test in future

integration-tests/load/ccip/ccip_test.go Outdated Show resolved Hide resolved
@0xAustinWang 0xAustinWang requested a review from a team as a code owner December 11, 2024 11:14
@0xAustinWang 0xAustinWang marked this pull request as draft December 11, 2024 11:18
@0xAustinWang 0xAustinWang force-pushed the aw/CCIPLoad branch 2 times, most recently from f4a9b8e to 87eab87 Compare January 10, 2025 04:49
@0xAustinWang 0xAustinWang changed the title CCIP Load Test connected to crib (not yet working) CCIP Load Test connected to crib Jan 15, 2025
@0xAustinWang
Copy link
Contributor Author

closing in favor of : #15954

@0xAustinWang 0xAustinWang deleted the aw/CCIPLoad branch January 16, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants