feat: Testground tooling #2456

evan-forbes · 2023-09-11T07:24:32Z

Overview

This is a refactor of the celestia-app tests in test-infra. The refactor focuses on further removing boiler plate and using our existing testing utilities for devUX, but also allows for more elaborate test scenarios and data collection.

Testground requires that each node is responsible for intializing, operating, performing tests, and collecting data on itself. To do this this refactor adds a new Role interface, where we define specific roles that Plan, Execute, and Retro. The first test case makes use of the interface to define a Leader and Follower, where we can have the Leader generate all the keys, configs, genesis, etc during Plan and then publishes that data to all Followers, which use their respective configuration to start their node. After execution of the test, each Role can run arbitrary Retro logic over the data produced or collected during the test run.

The first experiment is mean to add as similar environment to mainnet as possible. We will use this as a base control case for an upcoming PR that adds the unbounded blocksize test

Please see the README for more info and diagrams

closes #2033 blocking celestiaorg/celestia-core#945

Checklist

New and updated code has appropriate documentation
New and updated code has new and/or updated testing
Required CI checks are passing
Visual proof for any user facing features like CLI or documentation updates
Linked issues closed with keywords

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>

…b.com/celestiaorg/celestia-app into evan/multi-validator-genesis-creation

codecov-commenter · 2023-09-11T07:32:31Z

Codecov Report

Merging #2456 (0b4bf7a) into evan/multi-validator-genesis-creation (a383494) will increase coverage by 0.15%.
Report is 3 commits behind head on evan/multi-validator-genesis-creation.
The diff coverage is 100.00%.

@@                            Coverage Diff                            @@
##           evan/multi-validator-genesis-creation    #2456      +/-   ##
=========================================================================
+ Coverage                                  20.61%   20.76%   +0.15%     
=========================================================================
  Files                                        131      136       +5     
  Lines                                      15270    15624     +354     
=========================================================================
+ Hits                                        3148     3245      +97     
- Misses                                     11820    12055     +235     
- Partials                                     302      324      +22

Files Changed	Coverage Δ
test/genesis/accounts.go	`56.66% <ø> (ø)`
test/genesis/files.go	`0.00% <ø> (ø)`
test/genesis/genesis.go	`24.22% <100.00%> (ø)`
test/genesis/util.go	`77.77% <100.00%> (ø)`

... and 1 file with indirect coverage changes

evan-forbes · 2023-09-13T03:48:31Z

the test can run a 3 validator network atm, but there are still a few minor but blocking bugs left. I'll add a way to sync before the "planning" stage to handle those bugs, then we should be good write the larger tests 🤞

evan-forbes · 2023-12-06T16:46:51Z

test/testground/network/configurators.go

+
+var _ = Configurator(ConnectRandom(1))
+
+func ConnectRandom(numPeers int) Configurator {


these are deleted in the ubounded block size iirc, but current tests support them

evan-forbes · 2023-12-06T16:50:39Z

@rootulp do you have any clue why the ledger test is breaking? its breaking locally as well, but wasn't breaking a few commits ago

rootulp

I skipped reviewing the code in test/testground/* and instead focused on the celestia-app portions

rootulp · 2023-12-06T19:36:43Z

pkg/appconsts/testground/app_consts.go

+package testground
+
+const (
+	Version              uint64 = 420420420


[nit] Version could use a GoDoc that explains why this number looks crazy

resolved in 93cb5d4

rootulp · 2023-12-06T19:46:47Z

pkg/appconsts/versioned_consts.go

+	default:
+		return v1.SquareSizeUpperBound
+	}


IMO this thread shouldn't block this PR and instead we should create a new issue for this.

Go doesn't have exhaustive checking (like Rust) but I don't understand why silently defaulting to an old version is preferable over a panic. I see the options as:

Current

func SquareSizeUpperBound(v uint64) int { switch v { case testground.Version: return testground.SquareSizeUpperBound // There is currently only a single square size upper bound. default: return v1.SquareSizeUpperBound }

Proposal

func SquareSizeUpperBound(v uint64) int { switch v { case testground.Version: return testground.SquareSizeUpperBound case v1.Version: return v1.SquareSizeUpperBound case v2.Version: return v2.SquareSizeUpperBound default: panic(fmt.Sprintf("unsupported version %v", v)) } }

ref: https://rustc-dev-guide.rust-lang.org/pat-exhaustive-checking.html#pattern-and-exhaustiveness-checking

rootulp · 2023-12-06T19:48:27Z

pkg/appconsts/testground/app_consts.go

+const (
+	Version              uint64 = 420420420
+	SquareSizeUpperBound int    = 512
+	SubtreeRootThreshold int    = 64


[question] related to https://github.com/celestiaorg/celestia-app/pull/2456/files#r1417502424

why define this constant if it is never used?

resolved in 93cb5d4

we should probably do the same for v2

rootulp · 2023-12-06T19:52:53Z

test/testground/README.md

+// Role is the interface between a testground test entrypoint and the actual
+// test logic. Testground creates many instances and passes each instance a
+// configuration from the plan and manifest toml files. From those
+// configurations a Role is created for each node, and the three methods below
+// are ran in order.
+type Role interface {
+	// Plan is the first function called in a test by each node. It is
+	// responsible for creating the genesis block, configuring nodes, and
+	// starting the network.
+	Plan(ctx context.Context, runenv *runtime.RunEnv, initCtx *run.InitContext) error
+	// Execute is the second function called in a test by each node. It is
+	// responsible for running any experiments. This is phase where commands are
+	// sent and received.
+	Execute(ctx context.Context, runenv *runtime.RunEnv, initCtx *run.InitContext) error
+	// Retro is the last function called in a test by each node. It is
+	// responsible for collecting any data from the node and/or running any
+	// retrospective tests or benchmarks.
+	Retro(ctx context.Context, runenv *runtime.RunEnv, initCtx *run.InitContext) error
+}
+
+var _ Role = (*Leader)(nil)
+
+var _ Role = (*Follower)(nil)


[optional] This Go code may become stale with respect to the Role interface defined in test/testground/network.go. We may remove the code and replace with a permalink.

resolved in 93cb5d4

test/util/testnode/rpc_client.go

evan-forbes · 2023-12-08T15:48:48Z

There's a weird bug where this test is failing consistently in ci bug not locally. It also doesn't make any sense, since it was just passing and the commit that broke it only added docs and removed an usused var

evan-forbes · 2023-12-08T19:49:47Z

test/util/testnode/config.go

@@ -119,7 +124,8 @@ func DefaultConfig() *Config {
 		WithAppConfig(DefaultAppConfig()).
 		WithAppOptions(DefaultAppOptions()).
 		WithAppCreator(cmd.NewAppServer).
-		WithSupressLogs(true)
+		WithSuppressLogs(true).
+		WithConsensusParams(DefaultConsensusParams())


setting the default consensus params here fixes the failing test, which apparently we aren't doing doing on main! This doesn't fail on main, because we don't actually have a test that checks that the version is updated but now we do here

evan-forbes and others added 12 commits September 9, 2023 12:49

feat: add genesis test package for creating testnet genesises

a008844

feat!: use a specific genesis creation package

6e7cc58

chore: cleanup

86d492b

fix: update the default propose timeout

25d60cd

chore: cleanup

f4c8718

chore: linter

47e1a80

refactor!: change names to appease the linter

a383494

Apply suggestions from code review

39b306a

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>

fix: ci can't handle the club right now

e855a67

fix: ci can't handle the club right now

e16be41

Merge branch 'evan/multi-validator-genesis-creation' of https://githu…

e8b4926

…b.com/celestiaorg/celestia-app into evan/multi-validator-genesis-creation

feat: flesh out celestia-app's testground tests

eda02ec

evan-forbes added testing items that are strictly related to adding or extending test coverage refactor optional label for items that are related to implementation work and do not change functionality testground directly relevant to testground usage labels Sep 11, 2023

evan-forbes self-assigned this Sep 11, 2023

evan-forbes added 4 commits September 11, 2023 22:15

feat: finish first draft of testground tooling

e2de741

chore: cleanup

83f0f76

fix: go mod by importing specific version of app

db8d2f2

chore: update to use three validators

84f3980

evan-forbes requested review from Bidon15 and rach-id September 12, 2023 04:19

evan-forbes added 2 commits September 12, 2023 17:23

chore: update docker file

96fb988

fix: pass halt height

d39e44b

evan-forbes added 4 commits September 12, 2023 22:49

chore: more fixes

0ca3996

feat: sync statuses before beginning each test

cf50bb4

chore: deubbing

d8e2fed

chore: cleanup and switch back mnemonic

195383e

docs: add info about testground in the instructions

1b7ab87

evan-forbes commented Dec 6, 2023

View reviewed changes

fix: pin to ledger-cosmos-go v0.12.4 to fix ledger

d698845

evan-forbes requested review from staheri14, rootulp and Bidon15 December 6, 2023 17:49

rootulp previously approved these changes Dec 6, 2023

View reviewed changes

chore: reviewer feedback

93cb5d4

evan-forbes dismissed rootulp’s stale review via 93cb5d4 December 8, 2023 13:59

celestia-bot requested a review from a team December 8, 2023 13:59

evan-forbes requested a review from rootulp December 8, 2023 14:00

chore: sanity checking ci

39b36da

Merge branch 'main' into evan/testground-test

3d69fc0

rootulp previously approved these changes Dec 8, 2023

View reviewed changes

staheri14 previously approved these changes Dec 8, 2023

View reviewed changes

fix: test

50bc61e

evan-forbes dismissed stale reviews from staheri14 and rootulp via 50bc61e December 8, 2023 19:46

celestia-bot requested a review from a team December 8, 2023 19:46

evan-forbes requested a review from rootulp December 8, 2023 19:46

evan-forbes commented Dec 8, 2023

View reviewed changes

evan-forbes enabled auto-merge (squash) December 8, 2023 19:50

evan-forbes added 2 commits December 8, 2023 14:11

fix: update the expected version

ecbfb5e

fix: expected version

3b8d83f

rootulp approved these changes Dec 8, 2023

View reviewed changes

evan-forbes merged commit ce1c6cd into main Dec 8, 2023
28 checks passed

evan-forbes deleted the evan/testground-test branch December 8, 2023 20:35

rootulp mentioned this pull request Dec 8, 2023

test: versioned constant getters #2915

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Testground tooling #2456

feat: Testground tooling #2456

evan-forbes commented Sep 11, 2023 •

edited by coderabbitai bot

codecov-commenter commented Sep 11, 2023 •

edited

evan-forbes commented Sep 13, 2023

evan-forbes Dec 6, 2023

evan-forbes commented Dec 6, 2023

rootulp left a comment

rootulp Dec 6, 2023

evan-forbes Dec 8, 2023

rootulp Dec 6, 2023

rootulp Dec 6, 2023

evan-forbes Dec 8, 2023

rootulp Dec 6, 2023

evan-forbes Dec 8, 2023

evan-forbes commented Dec 8, 2023

evan-forbes Dec 8, 2023


		var _ = Configurator(ConnectRandom(1))

		func ConnectRandom(numPeers int) Configurator {

feat: Testground tooling #2456

feat: Testground tooling #2456

Conversation

evan-forbes commented Sep 11, 2023 • edited by coderabbitai bot

Overview

Checklist

codecov-commenter commented Sep 11, 2023 • edited

Codecov Report

evan-forbes commented Sep 13, 2023

Choose a reason for hiding this comment

evan-forbes commented Dec 6, 2023

rootulp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Current

Proposal

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evan-forbes commented Dec 8, 2023

Choose a reason for hiding this comment

evan-forbes commented Sep 11, 2023 •

edited by coderabbitai bot

codecov-commenter commented Sep 11, 2023 •

edited