From 820bc699f430e2d4d3f864bc9c21f42c5cba7d9b Mon Sep 17 00:00:00 2001
From: Jim Larson <jim@agoric.com>
Date: Thu, 18 Apr 2024 18:35:56 -0700
Subject: [PATCH] test: precedence of flag vs toml for abci client type

Also more documentation for TOML template.
---
 server/config/config.go |  3 ++
 server/config/toml.go   |  4 +-
 server/start_test.go    | 88 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/server/config/config.go b/server/config/config.go
index 8c7ffd899d04..ee57c6e048a8 100644
--- a/server/config/config.go
+++ b/server/config/config.go
@@ -104,6 +104,9 @@ type BaseConfig struct {
 	IAVLLazyLoading bool `mapstructure:"iavl-lazy-loading"`
 
 	// ABCIClientType selects the type of ABCI client.
+	// Valid settings are "committing" (default) or "local".
+	// The committing client allows greater query parallelism,
+	// but the local client is more defensive.
 	ABCIClientType string `mapstructure:"abci-client-type"`
 
 	// AppDBBackend defines the type of Database to use for the application and snapshots databases.
diff --git a/server/config/toml.go b/server/config/toml.go
index a53633cc7e3c..1c47ff1a2ebc 100644
--- a/server/config/toml.go
+++ b/server/config/toml.go
@@ -83,7 +83,9 @@ iavl-disable-fastnode = {{ .BaseConfig.IAVLDisableFastNode }}
 iavl-lazy-loading = {{ .BaseConfig.IAVLLazyLoading }}
 
 # ABCIClientType selects the type of ABCI client.
-# Default is "committing".
+# Valid settings are "committing" (default) or "local".
+# The committing client allows greater query parallelism,
+# but the local client is more defensive.
 abci-client-type = "{{ .BaseConfig.ABCIClientType }}"
 
 # AppDBBackend defines the database backend type to use for the application and snapshots DBs.
diff --git a/server/start_test.go b/server/start_test.go
index 8e546a9ace6d..ddbc88ccc4b4 100644
--- a/server/start_test.go
+++ b/server/start_test.go
@@ -1,9 +1,17 @@
 package server
 
 import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path"
 	"reflect"
 	"runtime"
 	"testing"
+
+	"github.com/spf13/cobra"
+	tmcfg "github.com/tendermint/tendermint/config"
 )
 
 func TestAbciClientType(t *testing.T) {
@@ -44,3 +52,83 @@ func TestAbciClientType(t *testing.T) {
 		})
 	}
 }
+
+var errCancelledInPreRun = errors.New("cancelled in prerun")
+
+func TestAbciClientPrecedence(t *testing.T) {
+	for i, tt := range []struct {
+		flag, toml, want string
+	}{
+		{
+			want: "committing",
+		},
+		{
+			flag: "foo",
+			want: "foo",
+		},
+		{
+			toml: "foo",
+			want: "foo",
+		},
+		{
+			flag: "foo",
+			toml: "bar",
+			want: "foo",
+		},
+	} {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			tempDir := t.TempDir()
+			err := os.Mkdir(path.Join(tempDir, "config"), os.ModePerm)
+			if err != nil {
+				t.Fatalf("creating config dir failed: %v", err)
+			}
+			appTomlPath := path.Join(tempDir, "config", "app.toml")
+
+			cmd := StartCmd(nil, tempDir)
+
+			if tt.flag != "" {
+				err = cmd.Flags().Set(FlagAbciClientType, tt.flag)
+				if err != nil {
+					t.Fatalf(`failed setting flag to "%s": %v`, tt.flag, err)
+				}
+			}
+
+			if tt.toml != "" {
+				writer, err := os.Create(appTomlPath)
+				if err != nil {
+					t.Fatalf(`failed creating "%s": %v`, appTomlPath, err)
+				}
+				_, err = writer.WriteString(fmt.Sprintf("%s = \"%s\"\n", FlagAbciClientType, tt.toml))
+				if err != nil {
+					t.Fatalf(`failed writing to app.toml: %v`, err)
+				}
+				err = writer.Close()
+				if err != nil {
+					t.Fatalf(`failed closing app.toml: %v`, err)
+				}
+			}
+
+			// Compare to tests in util_test.go
+			cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
+				err := InterceptConfigsPreRunHandler(cmd, "", nil, tmcfg.DefaultConfig())
+				if err != nil {
+					return err
+				}
+				return errCancelledInPreRun
+			}
+
+			serverCtx := NewDefaultContext()
+			ctx := context.WithValue(context.Background(), ServerContextKey, serverCtx)
+			err = cmd.ExecuteContext(ctx)
+			if err != errCancelledInPreRun {
+				t.Fatal(err)
+			}
+
+			gotClientType := serverCtx.Viper.GetString(FlagAbciClientType)
+
+			if gotClientType != tt.want {
+				t.Errorf(`want client type "%s", got "%s"`, tt.want, gotClientType)
+			}
+		})
+	}
+}