refactor: switch e2e tests to a binary #3301
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,83 @@ | ||||
| package main | ||||
|
|
||||
| import ( | ||||
| "context" | ||||
| "fmt" | ||||
| "log" | ||||
| "os" | ||||
| "time" | ||||
|
|
||||
| "github.com/celestiaorg/celestia-app/v2/pkg/appconsts" | ||||
| "github.com/celestiaorg/celestia-app/v2/test/e2e/testnets" | ||||
| "github.com/celestiaorg/celestia-app/v2/test/util/testnode" | ||||
| ) | ||||
|
|
||||
| const seed = 42 | ||||
|
|
||||
| func main() { | ||||
| if err := E2EThroughput(); err != nil { | ||||
| log.Fatalf("--- ERROR Throughput test: %v", err.Error()) | ||||
| } | ||||
| } | ||||
|
|
||||
| func E2EThroughput() error { | ||||
| os.Setenv("KNUU_NAMESPACE", "test") | ||||
|
|
||||
| latestVersion, err := testnets.GetLatestVersion() | ||||
| testnets.NoError("failed to get latest version", err) | ||||
|
|
||||
| log.Println("=== RUN E2EThroughput", "version:", latestVersion) | ||||
|
|
||||
| // create a new testnet | ||||
| testnet, err := testnets.New("E2EThroughput", seed, testnets.GetGrafanaInfoFromEnvVar()) | ||||
| testnets.NoError("failed to create testnet", err) | ||||
|
|
||||
| log.Println("Cleaning up testnet") | ||||
|
There was a problem hiding this comment. I expect this log will be printed immediately but the testnet will be cleaned up at the end of the test run. Proposal to move this log line inside the testnet.Cleanup() implementation and delete it here.
Suggested change
There was a problem hiding this comment. i think cleaning up testnet describes it correctly, since it implies that the process of cleaning up is ongoing. i'm happy to move it inside the cleanup though if you think it helps with readability/understanding. There was a problem hiding this comment. The process of cleaning up doesn't start until the end of the test because of the If you don't want to move the log statement, then you could defer testnet.Cleanup()
defer log.Println("Cleaning up testnet")There was a problem hiding this comment. In the original e2e throughput test (of the main branch), the method for informing the clean up of the testnet is different. Could you please elaborate on the reasons for this change from the original test? Here’s the specific section of the code for reference: celestia-app/test/e2e/throughput_test.go Lines 39 to 42 in a8e3085 There was a problem hiding this comment. What you could do is to wrap the testnet.Cleanup() call inside an anonymous function, and make it like defer func() {
t.Log("Cleaning up testnet")
testnet.Cleanup()}() |
||||
| defer testnet.Cleanup() | ||||
|
|
||||
| // add 2 validators | ||||
| testnets.NoError("failed to create genesis nodes", testnet.CreateGenesisNodes(2, latestVersion, 10000000, 0, testnets.DefaultResources)) | ||||
|
|
||||
| // obtain the GRPC endpoints of the validators | ||||
| gRPCEndpoints, err := testnet.RemoteGRPCEndpoints() | ||||
| testnets.NoError("failed to get validators GRPC endpoints", err) | ||||
| log.Println("validators GRPC endpoints", gRPCEndpoints) | ||||
|
|
||||
| // create txsim nodes and point them to the validators | ||||
| log.Println("Creating txsim nodes") | ||||
| // version of the txsim docker image to be used | ||||
| txsimVersion := "a92de72" | ||||
|
Comment on lines
+48
to
+49
There was a problem hiding this comment. [optional] this could be extracted to a constant outside the test like we do in some other tests. There was a problem hiding this comment. i agree, although i tried to keep to the scope of this issue and not change the original tests too much. I think we could do more work to make e2e tests generally better and more readable in another pr. I'm also not sure if we need to refactor performance/experimental tests since they're isolated and not even run as part of the e2e test suite. There was a problem hiding this comment.
There is an issue concerning the replacement of this hardcoded version: #3288. However, I agree that in its current state, it could be replaced by a constant. As I am actively working on the throughput tests, I can address this in my upcoming PRs. I've created a tracking issue for now to keep it on the radar: #3343. |
||||
|
|
||||
| err = testnet.CreateTxClients(txsimVersion, 1, "10000-10000", testnets.DefaultResources, gRPCEndpoints) | ||||
| testnets.NoError("failed to create tx clients", err) | ||||
|
|
||||
| // start the testnet | ||||
| log.Println("Setting up testnet") | ||||
| testnets.NoError("failed to setup testnet", testnet.Setup()) | ||||
| log.Println("Starting testnet") | ||||
| testnets.NoError("failed to start testnet", testnet.Start()) | ||||
|
|
||||
| // once the testnet is up, start the txsim | ||||
| log.Println("Starting txsim nodes") | ||||
| testnets.NoError("failed to start tx clients", testnet.StartTxClients()) | ||||
|
|
||||
| // wait some time for the txsim to submit transactions | ||||
| time.Sleep(1 * time.Minute) | ||||
|
|
||||
| log.Println("Reading blockchain") | ||||
| blockchain, err := testnode.ReadBlockchain(context.Background(), testnet.Node(0).AddressRPC()) | ||||
| testnets.NoError("failed to read blockchain", err) | ||||
|
|
||||
| totalTxs := 0 | ||||
| for _, block := range blockchain { | ||||
| if appconsts.LatestVersion != block.Version.App { | ||||
| return fmt.Errorf("expected app version %d, got %d", appconsts.LatestVersion, block.Version.App) | ||||
| } | ||||
| totalTxs += len(block.Data.Txs) | ||||
| } | ||||
| if totalTxs < 10 { | ||||
| return fmt.Errorf("expected at least 10 transactions, got %d", totalTxs) | ||||
| } | ||||
| log.Println("--- PASS ✅: E2EThroughput") | ||||
|
There was a problem hiding this comment. Liked the graphical indication for successful tests 👍 |
||||
| return nil | ||||
| } | ||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆒 I think this enables developers to pass extra arguments to
make test-e2eand they get plumbed in togo run...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, yes