implement @Wondertan's suggestion to let empty pass all the way and p…

…refer fail on validation, also fill out tests for Validate

nodebuilder/core/config.go

@@ -37,6 +37,13 @@ func (cfg *Config) Validate() error {
 		return nil
 	}
 
+	if cfg.RPCPort == "" {
+		return fmt.Errorf("nodebuilder/core: rpc port is not set")
+	}
+	if cfg.GRPCPort == "" {
+		return fmt.Errorf("nodebuilder/core: grpc port is not set")
+	}
+
 	ip, err := utils.ValidateAddr(cfg.IP)
 	if err != nil {
 		return err

nodebuilder/core/config_test.go

@@ -0,0 +1,84 @@
+package core
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidate(t *testing.T) {
+	tests := []struct {
+		name      string
+		cfg       Config
+		expectErr bool
+	}{
+		{
+			name: "valid config",
+			cfg: Config{
+				IP:       "127.0.0.1",
+				RPCPort:  DefaultRPCPort,
+				GRPCPort: DefaultGRPCPort,
+			},
+			expectErr: false,
+		},
+		{
+			name:      "empty config, no endpoint",
+			cfg:       Config{},
+			expectErr: false,
+		},
+		{
+			name: "missing RPC port",
+			cfg: Config{
+				IP:       "127.0.0.1",
+				GRPCPort: DefaultGRPCPort,
+			},
+			expectErr: true,
+		},
+		{
+			name: "missing GRPC port",
+			cfg: Config{
+				IP:      "127.0.0.1",
+				RPCPort: DefaultRPCPort,
+			},
+			expectErr: true,
+		},
+		{
+			name: "invalid IP",
+			cfg: Config{
+				IP:       "invalid-ip",
+				RPCPort:  DefaultRPCPort,
+				GRPCPort: DefaultGRPCPort,
+			},
+			expectErr: true,
+		},
+		{
+			name: "invalid RPC port",
+			cfg: Config{
+				IP:       "127.0.0.1",
+				RPCPort:  "invalid-port",
+				GRPCPort: DefaultGRPCPort,
+			},
+			expectErr: true,
+		},
+		{
+			name: "invalid GRPC port",
+			cfg: Config{
+				IP:       "127.0.0.1",
+				RPCPort:  DefaultRPCPort,
+				GRPCPort: "invalid-port",
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.cfg.Validate()
+			if tt.expectErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

nodebuilder/core/flags.go

@@ -50,14 +50,11 @@ func ParseFlags(
 		return nil
 	}
 
-	// in the case that the RPC port is not set, use the default port
-	// or in the case a value has been set via flag,
-	// set that as highest precedence
-	if cfg.RPCPort == "" || cmd.Flag(coreRPCFlag).Changed {
+	if cmd.Flag(coreRPCFlag).Changed {
 		rpc := cmd.Flag(coreRPCFlag).Value.String()
 		cfg.RPCPort = rpc
 	}
-	if cfg.GRPCPort == "" || cmd.Flag(coreGRPCFlag).Changed {
+	if cmd.Flag(coreGRPCFlag).Changed {
 		grpc := cmd.Flag(coreGRPCFlag).Value.String()
 		cfg.GRPCPort = grpc
 	}

nodebuilder/core/flags_test.go

@@ -49,7 +49,7 @@ func TestParseFlags(t *testing.T) {
 				RPCPort:  DefaultRPCPort,
 				GRPCPort: DefaultGRPCPort,
 			},
-			expectError: false,
+			expectError: true,
 		},
 		{
 			name: "no flags, values from input config.toml ",