ci,test: retain data in failed integ/acct tests, test fixes

This will skip removal of the temporary folder for root_dir if
the test fails.  The containers are still stopped and removed.

This also sets the race detector options to kill kwild when a race
is detected, and to write races to a file in the retained root_dir
instead of stderr, which would be inaccessible after the containers
are stopped and removed.

Finally, this logs to kwild.log for integration tests too. It was
already doing this for acceptance tests.

test/acceptance: remove messy flag

test: use require in validator tests when needed to avoid panics

ci: use tagged golangci-lint-action

tests: mitigate premature join expiry

tests: wait for sync on other nodes

fix error check in validator join expiry

.github/workflows/ci.yaml

@@ -84,7 +84,7 @@ jobs:
           go build -mod=readonly ./... ./parse/... ./core/... ./test/specifications/
 
       - name: Lint
-        uses: golangci/golangci-lint-action@3cfe3a4abbb849e10058ce4af15d205b6da42804
+        uses: golangci/golangci-lint-action@v4.0.0
         with:
           install-mode: "binary"
           version: "latest"
@@ -170,6 +170,7 @@ jobs:
             git_commit=${{ github.sha }}
             version=${{ env.GIT_TAG }}
             build_time=${{ env.BUILD_TIME }}
+          #  go_race=-race
           file: ./build/package/docker/kwild.dockerfile
           push: false
           tags: kwild:latest

Taskfile.yml

@@ -157,7 +157,7 @@ tasks:
     cmds:
       - task: dev:up:nb
 
-  dev:up:nb: # run `task dev:up:nb -- -messy` if you don't want cleanup
+  dev:up:nb:
     desc: Start the dev environment without rebuilding docker image
     env:
       # NOTE: this timeout should be long enough to attach to debugger
@@ -196,7 +196,6 @@ tasks:
   # e.g.
   # - task test:act:nb -- -remote
   # - task test:act:nb -- -drivers grpc
-  # - task test:act:nb -- -messy
   # - task test:act:nb -- -parallel-mode
   test:act:nb:
     desc: Run acceptance tests without building docker image

cmd/kwil-admin/nodecfg/generate.go

@@ -267,6 +267,7 @@ func GenerateTestnetConfig(genCfg *TestnetGenerateConfig, opts *ConfigOpts) erro
 			return fmt.Errorf("failed to merge config file %s: %w", genCfg.ConfigFile, err)
 		}
 	}
+	cfg.Logging.OutputPaths = append(cfg.Logging.OutputPaths, "kwild.log")
 
 	privateKeys := make([]cmtEd.PrivKey, nNodes)
 	for i := range privateKeys {

deployments/compose/kwil/single/docker-compose.yml

@@ -23,7 +23,7 @@ services:
       -c max_wal_senders=10
       -c max_replication_slots=10
       -c max_prepared_transactions=2
-    volumes: 
+    volumes:
       - pgkwil:/var/lib/postgresql/data
       - ./init.sql:/docker-entrypoint-initdb.d/create_user.sql
     networks:
@@ -75,4 +75,4 @@ networks:
     ipam:
       driver: default
       config:
-        - subnet: 172.5.100.0/16
+        - subnet: 172.5.100.0/23

test/acceptance/docker-compose-dev.yml

@@ -14,6 +14,8 @@ services:
       - "40000:40000" # debugger, if build with debug dockerfile
     #env_file:
       # NOTE: docker compose by default will use `.env` file if presented
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnode}

test/acceptance/docker-compose.yml

@@ -12,6 +12,8 @@ services:
       - "40000" # debugger, if build with debug dockerfile
     #env_file:
       # NOTE: docker compose by default will use `.env` file if presented
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnode}

test/acceptance/helper.go

@@ -46,9 +46,6 @@ type ActTestCfg struct {
 	SchemaFile                string
 	DockerComposeFile         string
 	DockerComposeOverrideFile string
-	// NoCleanup is used to keep the test environment from being cleaned up
-	// i.e. config files, logs, containers
-	NoCleanup bool
 
 	WaitTimeout time.Duration
 	LogLevel    string
@@ -148,9 +145,6 @@ func (r *ActHelper) LoadConfig() *ActTestCfg {
 	cfg.GasEnabled, err = strconv.ParseBool(getEnv("KACT_GAS_ENABLED", "false"))
 	require.NoError(r.t, err, "invalid gasEnabled bool")
 
-	cfg.NoCleanup, err = strconv.ParseBool(getEnv("KACT_NO_CLEANUP", "false"))
-	require.NoError(r.t, err, "invalid noCleanup bool")
-
 	// value is in format of "10s" or "1m"
 	waitTimeout := getEnv("KACT_WAIT_TIMEOUT", "10s")
 	cfg.WaitTimeout, err = time.ParseDuration(waitTimeout)
@@ -176,16 +170,17 @@ func (r *ActHelper) updateEnv(k, v string) {
 
 func (r *ActHelper) generateNodeConfig() {
 	r.t.Logf("generate node config")
-	var tmpPath string
-	if r.cfg.NoCleanup {
-		var err error
-		tmpPath, err = os.MkdirTemp("", "TestKwilAct")
-		if err != nil {
-			r.t.Fatal(err)
-		}
-	} else {
-		tmpPath = r.t.TempDir() // automatically removed by testing.T.Cleanup
+	tmpPath, err := os.MkdirTemp("", "TestKwilAct")
+	if err != nil {
+		r.t.Fatal(err)
 	}
+	r.t.Cleanup(func() {
+		if r.t.Failed() {
+			r.t.Logf("Retaining data for failed test at path %v", tmpPath)
+			return
+		}
+		os.RemoveAll(tmpPath)
+	})
 
 	r.t.Logf("created test temp directory: %s", tmpPath)
 
@@ -195,7 +190,7 @@ func (r *ActHelper) generateNodeConfig() {
 	}
 	creatorIdent := hex.EncodeToString(r.cfg.CreatorSigner.Identity())
 
-	err := nodecfg.GenerateNodeConfig(&nodecfg.NodeGenerateConfig{
+	err = nodecfg.GenerateNodeConfig(&nodecfg.NodeGenerateConfig{
 		ChainID:       TestChainID,
 		BlockInterval: time.Second,
 		// InitialHeight: 0,
@@ -229,13 +224,11 @@ func (r *ActHelper) runDockerCompose(ctx context.Context) {
 	dc, err := compose.NewDockerCompose(composeFiles...)
 	require.NoError(r.t, err, "failed to create docker compose object for single kwild node")
 
-	if !r.cfg.NoCleanup {
-		r.t.Cleanup(func() {
-			r.t.Logf("teardown docker compose")
-			err := dc.Down(ctx, compose.RemoveOrphans(true), compose.RemoveImagesLocal, compose.RemoveVolumes(true))
-			require.NoErrorf(r.t, err, "failed to teardown %s", dc.Services())
-		})
-	}
+	r.t.Cleanup(func() {
+		r.t.Logf("teardown docker compose")
+		err := dc.Down(ctx, compose.RemoveOrphans(true), compose.RemoveImagesLocal, compose.RemoveVolumes(true))
+		require.NoErrorf(r.t, err, "failed to teardown %s", dc.Services())
+	})
 
 	// NOTE: if you run with debugger image, you need to attach to the debugger
 	// before the timeout

test/acceptance/kwild_test.go

@@ -15,7 +15,6 @@ import (
 
 var dev = flag.Bool("dev", false, "run for development purpose (no tests)")
 var remote = flag.Bool("remote", false, "test against remote node")
-var noCleanup = flag.Bool("messy", false, "do not cleanup test directories or stop the docker compose when done")
 
 // NOTE: `-parallel` is a flag that is already used by `go test`
 var parallelMode = flag.Bool("parallel-mode", false, "run tests in parallelMode mode")
@@ -34,10 +33,6 @@ func TestLocalDevSetup(t *testing.T) {
 	cfg.DockerComposeFile = "./docker-compose-dev.yml" // use the dev compose file
 	cfg.GasEnabled = true
 
-	if *noCleanup {
-		cfg.NoCleanup = true
-	}
-
 	helper.Setup(ctx)
 	helper.WaitUntilInterrupt()
 }
@@ -60,9 +55,6 @@ func TestKwildTransferAcceptance(t *testing.T) {
 			helper := acceptance.NewActHelper(t)
 			cfg := helper.LoadConfig()
 			cfg.GasEnabled = true
-			if *noCleanup {
-				cfg.NoCleanup = true
-			}
 			if !*remote {
 				helper.Setup(ctx)
 			}

test/go.mod

@@ -22,7 +22,6 @@ require (
 	github.com/testcontainers/testcontainers-go v0.27.0
 	github.com/testcontainers/testcontainers-go/modules/compose v0.27.0
 	go.uber.org/zap v1.26.0
-	google.golang.org/grpc v1.60.0
 )
 
 require (
@@ -242,6 +241,7 @@ require (
 	google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20231106174013-bbf56f31fb17 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20231120223509-83a465c0220f // indirect
+	google.golang.org/grpc v1.60.0 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

test/integration/docker-compose.yml.template

@@ -11,6 +11,8 @@ services:
       - "26657"
     #env_file:
       # NOTE: docker compose by default will use `.env` file if presented
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node0
@@ -68,6 +70,8 @@ services:
       - "50051"
       - "26656"
       - "26657"
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node1
@@ -125,6 +129,8 @@ services:
       - "50051"
       - "26656"
       - "26657"
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node2
@@ -185,6 +191,8 @@ services:
       - "50051"
       - "26656"
       - "26657"
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node3
@@ -242,6 +250,8 @@ services:
       - "50051"
       - "26656"
       - "26657"
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node4
@@ -299,6 +309,8 @@ services:
       - "50051"
       - "26656"
       - "26657"
+    environment:
+      GORACE: "halt_on_error=1 log_path=/app/kwil/datarace"
     volumes:
       - type: bind
         source: ${KWIL_HOME:-./.testnet}/node5

test/integration/helper.go

@@ -433,7 +433,18 @@ func (r *IntHelper) RunDockerComposeWithServices(ctx context.Context, services [
 // 4. Generate node configuration files
 // 5. Run docker-compose with the given services
 func (r *IntHelper) Setup(ctx context.Context, services []string) {
-	tmpDir := r.t.TempDir()
+	tmpDir, err := os.MkdirTemp("", "TestKwilInt")
+	if err != nil {
+		r.t.Fatal(err)
+	}
+	r.t.Cleanup(func() {
+		if r.t.Failed() {
+			r.t.Logf("Retaining data for failed test at path %v", tmpDir)
+			return
+		}
+		os.RemoveAll(tmpDir)
+	})
+
 	r.t.Logf("create test directory: %s", tmpDir)
 
 	r.prepareDockerCompose(ctx, tmpDir)

test/integration/kwild_test.go

@@ -78,6 +78,8 @@ func TestKwildDatabaseIntegration(t *testing.T) {
 
 			// Create a new database and verify that the database exists on other nodes
 			specifications.DatabaseDeploySpecification(ctx, t, node0Driver)
+			// TODO: wait for node 1 and 2 to hit whatever height 0 is at
+			time.Sleep(2 * time.Second)
 			specifications.DatabaseVerifySpecification(ctx, t, node1Driver, true)
 			specifications.DatabaseVerifySpecification(ctx, t, node2Driver, true)
 
@@ -143,7 +145,7 @@ func TestKwildValidatorUpdatesIntegration(t *testing.T) {
 
 	ctx := context.Background()
 
-	const expiryBlocks = 10
+	const expiryBlocks = 20
 	const blockInterval = time.Second
 	const numVals, numNonVals = 3, 1
 	opts := []integration.HelperOpt{
@@ -154,7 +156,7 @@ func TestKwildValidatorUpdatesIntegration(t *testing.T) {
 		integration.WithGas(), // must give the joining node some gas too
 	}
 
-	expiryWait := 2 * expiryBlocks * blockInterval
+	const expiryWait = 3 * expiryBlocks * blockInterval / 2
 
 	testDrivers := strings.Split(*drivers, ",")
 	for _, driverType := range testDrivers {
@@ -190,6 +192,7 @@ func TestKwildValidatorUpdatesIntegration(t *testing.T) {
 			 - Consensus reached, Node3 is a Validator
 			*/
 			specifications.ValidatorNodeJoinSpecification(ctx, t, joinerDriver, joinerPubKey, 3)
+			time.Sleep(2 * time.Second)
 			// Node 0,1 approves
 			specifications.ValidatorNodeApproveSpecification(ctx, t, node0Driver, joinerPubKey, 3, 3, false)
 			specifications.ValidatorNodeApproveSpecification(ctx, t, node1Driver, joinerPubKey, 3, 4, true)
@@ -206,6 +209,7 @@ func TestKwildValidatorUpdatesIntegration(t *testing.T) {
 			 Rejoin: (same as join process)
 			*/
 			specifications.ValidatorNodeJoinSpecification(ctx, t, joinerDriver, joinerPubKey, 3)
+			time.Sleep(2 * time.Second)
 			// Node 0, 1 approves
 			specifications.ValidatorNodeApproveSpecification(ctx, t, node0Driver, joinerPubKey, 3, 3, false)
 			specifications.ValidatorNodeApproveSpecification(ctx, t, node1Driver, joinerPubKey, 3, 4, true)