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

Integration test Setup Nodes and Add DON [CCIP-2810] #1190

Merged
merged 11 commits into from
Jul 16, 2024

Conversation

asoliman92
Copy link
Contributor

@asoliman92 asoliman92 commented Jul 12, 2024

Setting up a basic test that have basic OCR3 Nodes. Each Node can access 3 EVM chains that are created and connected to each others. Check PR for more details on creating the chains and connecting them.

As JobSpecs for CCIP are not ready. The main goal of this PR is to have the nodes and AddDon on capability registry for these nodes for each chain. Once JobSpec is ready we'll be able to test that the nodes picks up the changes and deploys the plugins, start sending, receiving,..etc.

@@ -1,4 +1,4 @@
package home_chain
package ccip_integration_tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving the file up one directory, no need to review changes.

@asoliman92 asoliman92 force-pushed the integration_test/ocr_nodes branch from 665cf9b to 85f3692 Compare July 12, 2024 16:39
@asoliman92 asoliman92 marked this pull request as ready for review July 12, 2024 16:39
@asoliman92 asoliman92 requested a review from a team as a code owner July 12, 2024 16:39
tickCtx, tickCancel := context.WithCancel(testutils.Context(t))
defer tickCancel()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this'll work, tickCancel() will be called likely before the goroutine is scheduled. You can keep it in the Cleanup though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't even think you need this extra ctx, you can just watch Done() on testutils.Context(t)

Copy link
Contributor

@dimkouv dimkouv left a comment

Choose a reason for hiding this comment

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

Hmm there might be something interesting.

func TestFlk(t *testing.T) {
	for i := 0; i < 20; i++ {
		TestIntegration_OCR3Nodes(t)
	}
}

Can you try to see if this ^ fails?

I believe there's some db resource that needs to be cleaned up after the test finishes.

@asoliman92
Copy link
Contributor Author


	for i := 0; i < 20; i++ {
		TestIntegration_OCR3Nodes(t)
	}
}

I tested 100 times and it passed them all. Do you get any issues locally?

@asoliman92 asoliman92 enabled auto-merge (squash) July 16, 2024 09:04
@asoliman92 asoliman92 merged commit 7d2cae3 into ccip-develop Jul 16, 2024
102 checks passed
@asoliman92 asoliman92 deleted the integration_test/ocr_nodes branch July 16, 2024 09:12
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.

4 participants