fixup: Respond to review feedback

Author: maru-ava

tests/fixture/e2e/env.go

@@ -84,8 +84,11 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 
 	var network *tmpnet.Network
 
+	networkCmd, err := flagVars.NetworkCmd()
+	require.NoError(err)
+
 	// Consider monitoring flags for any command but stop
-	if !flagVars.StopNetwork() {
+	if networkCmd != StopNetworkCmd {
 		if flagVars.StartCollectors() {
 			require.NoError(tmpnet.StartCollectors(tc.DefaultContext(), tc.Log()))
 		}
@@ -103,8 +106,8 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 		}
 	}
 
-	// Need to load the network if it is being stopped or reused
-	if flagVars.StopNetwork() || flagVars.ReuseNetwork() {
+	// Attempt to load the network if it may already be running
+	if networkCmd == StopNetworkCmd || networkCmd == ReuseNetworkCmd || networkCmd == RestartNetworkCmd {
 		networkDir := flagVars.NetworkDir()
 		var networkSymlink string // If populated, prompts removal of the referenced symlink if --stop-network is specified
 		if len(networkDir) == 0 {
@@ -130,7 +133,7 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 			)
 		}
 
-		if flagVars.StopNetwork() {
+		if networkCmd == StopNetworkCmd {
 			if len(networkSymlink) > 0 {
 				// Remove the symlink to avoid attempts to reuse the stopped network
 				tc.Log().Info("removing symlink",
@@ -147,8 +150,9 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 				tc.Log().Warn("no network to stop")
 			}
 			os.Exit(0)
-		} else if network != nil && flagVars.RestartNetwork() {
-			// A network is only restarted if it is already running and stop was not requested
+		}
+
+		if network != nil && networkCmd == RestartNetworkCmd {
 			require.NoError(network.Restart(tc.DefaultContext(), tc.Log()))
 		}
 	}
@@ -168,8 +172,7 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 			network,
 			flagVars.RootNetworkDir(),
 			flagVars.NetworkShutdownDelay(),
-			flagVars.StartNetwork(),
-			flagVars.ReuseNetwork(),
+			networkCmd,
 		)
 	}
 
@@ -178,7 +181,7 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 		require.NoError(tmpnet.WaitForPromtailReadiness(tc.DefaultContext(), tc.Log()))
 	}
 
-	if flagVars.StartNetwork() {
+	if networkCmd == StartNetworkCmd {
 		os.Exit(0)
 	}
 
@@ -241,7 +244,6 @@ func (te *TestEnvironment) StartPrivateNetwork(network *tmpnet.Network) {
 		network,
 		te.RootNetworkDir,
 		te.PrivateNetworkShutdownDelay,
-		false, /* skipShutdown */
-		false, /* reuseNetwork */
+		EmptyNetworkCmd,
 	)
 }

tests/fixture/e2e/flags.go

@@ -4,6 +4,7 @@
 package e2e
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"os"
@@ -15,6 +16,16 @@ import (
 	"github.com/ava-labs/avalanchego/tests/fixture/tmpnet/flags"
 )
 
+type NetworkCmd int
+
+const (
+	EmptyNetworkCmd NetworkCmd = iota
+	StartNetworkCmd
+	StopNetworkCmd
+	RestartNetworkCmd
+	ReuseNetworkCmd
+)
+
 type FlagVars struct {
 	startNetwork     bool
 	startNetworkVars *flags.StartNetworkVars
@@ -24,14 +35,36 @@ type FlagVars struct {
 
 	networkDir     string
 	reuseNetwork   bool
-	restartNetwork bool
 	stopNetwork    bool
+	restartNetwork bool
 
 	activateFortuna bool
 }
 
-func (v *FlagVars) StartNetwork() bool {
-	return v.startNetwork
+func (v *FlagVars) NetworkCmd() (NetworkCmd, error) {
+	cmd := EmptyNetworkCmd
+	count := 0
+	if v.startNetwork {
+		cmd = StartNetworkCmd
+		count++
+	}
+	if v.stopNetwork {
+		cmd = StopNetworkCmd
+		count++
+	}
+	if v.restartNetwork {
+		cmd = RestartNetworkCmd
+		count++
+	}
+	if v.reuseNetwork {
+		cmd = ReuseNetworkCmd
+		count++
+	}
+	if count > 1 {
+		return EmptyNetworkCmd, errors.New("only one of --start-network, --stop-network, --restart-network, or --reuse-network can be specified")
+	}
+
+	return cmd, nil
 }
 
 func (v *FlagVars) RootNetworkDir() string {
@@ -68,18 +101,6 @@ func (v *FlagVars) NetworkDir() string {
 	return os.Getenv(tmpnet.NetworkDirEnvName)
 }
 
-func (v *FlagVars) ReuseNetwork() bool {
-	return v.reuseNetwork
-}
-
-func (v *FlagVars) RestartNetwork() bool {
-	return v.restartNetwork
-}
-
-func (v *FlagVars) StopNetwork() bool {
-	return v.stopNetwork
-}
-
 func (v *FlagVars) NetworkShutdownDelay() time.Duration {
 	if v.startCollectors {
 		// Only return a non-zero value if we want to ensure the collectors have
@@ -104,7 +125,7 @@ func RegisterFlagsWithDefaultOwner(defaultOwner string) *FlagVars {
 		&vars.startNetwork,
 		"start-network",
 		false,
-		"[optional] start a new network and exit without executing any tests. The new network cannot be reused with --reuse-network. Ignored if either --reuse-network or --stop-network is provided.",
+		"[optional] start a new network and exit without executing any tests. The new network cannot be reused with --reuse-network.",
 	)
 
 	vars.startNetworkVars = flags.NewStartNetworkFlagVars(defaultOwner)
@@ -117,22 +138,22 @@ func RegisterFlagsWithDefaultOwner(defaultOwner string) *FlagVars {
 	flag.StringVar(
 		&vars.networkDir,
 		"network-dir",
-		"",
+		tmpnet.GetEnvWithDefault(tmpnet.NetworkDirEnvName, ""),
 		fmt.Sprintf("[optional] the dir containing the configuration of an existing network. Will only be used if --reuse-network, --restart-network or --stop-network are specified. Also possible to configure via the %s env variable.", tmpnet.NetworkDirEnvName),
 	)
 
 	flag.BoolVar(
 		&vars.reuseNetwork,
 		"reuse-network",
 		false,
-		"[optional] reuse an existing network previously started with --reuse-network. If a network is not already running, create a new one and leave it running for subsequent usage. Ignored if --stop-network is provided.",
+		"[optional] run tests against an existing network previously started with --reuse-network. If a network is not already running, create a new one and leave it running for subsequent usage.",
 	)
 
 	flag.BoolVar(
 		&vars.restartNetwork,
 		"restart-network",
 		false,
-		"[optional] restart an existing network previously started with --reuse-network. Useful for ensuring a network is running with the current state of binaries on disk. Ignored if a network is not already running or --stop-network is provided.",
+		"[optional] like --reuse-network except an already running network is restarted before running tests to ensure the network represents the current state of binaries on disk.",
 	)
 
 	flag.BoolVar(

tests/fixture/e2e/helpers.go

@@ -270,8 +270,7 @@ func StartNetwork(
 	network *tmpnet.Network,
 	rootNetworkDir string,
 	shutdownDelay time.Duration,
-	skipShutdown bool,
-	reuseNetwork bool,
+	networkCmd NetworkCmd,
 ) {
 	require := require.New(tc)
 
@@ -296,7 +295,7 @@ func StartNetwork(
 	symlinkPath, err := tmpnet.GetReusableNetworkPathForOwner(network.Owner)
 	require.NoError(err)
 
-	if reuseNetwork {
+	if networkCmd == ReuseNetworkCmd || networkCmd == RestartNetworkCmd {
 		// Symlink the path of the created network to the default owner path (e.g. latest_avalanchego-e2e)
 		// to enable easy discovery for reuse.
 		require.NoError(os.Symlink(network.Dir, symlinkPath))
@@ -307,16 +306,16 @@ func StartNetwork(
 	}
 
 	tc.DeferCleanup(func() {
-		if reuseNetwork {
+		if networkCmd == ReuseNetworkCmd || networkCmd == RestartNetworkCmd {
 			tc.Log().Info("skipping shutdown for network intended for reuse",
 				zap.String("networkDir", network.Dir),
 				zap.String("symlinkPath", symlinkPath),
 			)
 			return
 		}
 
-		if skipShutdown {
-			tc.Log().Info("skipping shutdown for network",
+		if networkCmd == StartNetworkCmd {
+			tc.Log().Info("skipping shutdown for --start-network",
 				zap.String("networkDir", network.Dir),
 			)
 			return

tests/fixture/tmpnet/flags/common.go

@@ -3,12 +3,7 @@
 
 package flags
 
-// The following function signatures are common across flag and
-// spf13/pflag. They can be used to define a single registration method that
-// supports both flag libraries.
-
-type stringVarFunc func(p *string, name string, value string, usage string)
-
-type boolVarFunc func(p *bool, name string, value bool, usage string)
-
-type intVarFunc func(p *int, name string, value int, usage string)
+// The following function signature is common across flag and
+// spf13/pflag. It can be used to define a single registration method
+// that supports both flag libraries.
+type varFunc[T any] func(p *T, name string, value T, usage string)

tests/fixture/tmpnet/flags/process_runtime.go

@@ -35,7 +35,7 @@ func (v *processRuntimeVars) registerWithFlagSet(flagSet *pflag.FlagSet) {
 	v.register(flagSet.StringVar, flagSet.BoolVar)
 }
 
-func (v *processRuntimeVars) register(stringVar stringVarFunc, boolVar boolVarFunc) {
+func (v *processRuntimeVars) register(stringVar varFunc[string], boolVar varFunc[bool]) {
 	stringVar(
 		&v.config.AvalancheGoPath,
 		avalanchegoPathFlag,

tests/fixture/tmpnet/flags/runtime.go

@@ -37,7 +37,7 @@ func NewRuntimeConfigFlagSetVars(flagSet *pflag.FlagSet) *RuntimeConfigVars {
 	return v
 }
 
-func (v *RuntimeConfigVars) register(stringVar stringVarFunc) {
+func (v *RuntimeConfigVars) register(stringVar varFunc[string]) {
 	stringVar(
 		&v.runtime,
 		"runtime",

tests/fixture/tmpnet/flags/start_network.go

@@ -38,7 +38,7 @@ func NewStartNetworkFlagSetVars(flagSet *pflag.FlagSet, defaultNetworkOwner stri
 	return v
 }
 
-func (v *StartNetworkVars) register(stringVar stringVarFunc, intVar intVarFunc) {
+func (v *StartNetworkVars) register(stringVar varFunc[string], intVar varFunc[int]) {
 	stringVar(
 		&v.RootNetworkDir,
 		"root-network-dir",

tests/fixture/tmpnet/tmpnetctl/main.go

@@ -109,7 +109,7 @@ func main() {
 			return nil
 		},
 	}
-	startNetworkVars = flags.NewStartNetworkFlagSetVars(startNetworkCmd.PersistentFlags(), "")
+	startNetworkVars = flags.NewStartNetworkFlagSetVars(startNetworkCmd.PersistentFlags(), "" /* defaultNetworkOwner */)
 	rootCmd.AddCommand(startNetworkCmd)
 
 	stopNetworkCmd := &cobra.Command{

tests/upgrade/upgrade_test.go

@@ -85,8 +85,7 @@ var _ = ginkgo.Describe("[Upgrade]", func() {
 			network,
 			"", /* rootNetworkDir */
 			shutdownDelay,
-			false, /* skipShutdown */
-			false, /* reuseNetwork */
+			e2e.EmptyNetworkCmd,
 		)
 
 		tc.By(fmt.Sprintf("restarting all nodes with %q binary", avalancheGoExecPathToUpgradeTo))