Server tests pass!

coder · ammario · Apr 7, 2023 · Mar 30, 2023 · Mar 30, 2023 · Mar 30, 2023
commit 0af47bbf1dd2cc462fa85b45effd6d2f58bf6bca
diff --git a/cli/clibase/values.go b/cli/clibase/values.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"reflect"
 	"strconv"
 	"strings"
 	"time"
@@ -350,6 +351,12 @@ func (s *Struct[T]) MarshalYAML() (interface{}, error) {
 }
 
 func (s *Struct[T]) UnmarshalYAML(n *yaml.Node) error {
+	// HACK: for compatibility with flags, we set the value to nil if the node
+	// is empty and T is a slice.
+	if typ := reflect.TypeOf(s.Value); typ.Kind() == reflect.Slice && len(n.Content) == 0 {
+		reflect.ValueOf(&s.Value).Elem().Set(reflect.Zero(typ))
+		return nil
+	}
 	return n.Decode(&s.Value)
 }
 

diff --git a/cli/clibase/yaml.go b/cli/clibase/yaml.go
@@ -201,6 +201,12 @@ func (o *Option) setFromYAMLNode(n *yaml.Node) error {
 	case yaml.ScalarNode:
 		return o.Value.Set(n.Value)
 	case yaml.SequenceNode:
+		// We treat empty values as nil for consistency with other option
+		// mechanisms.
+		if len(n.Content) == 0 {
+			o.Value = nil
+			return nil
+		}
 		return n.Decode(o.Value)
 	case yaml.MappingNode:
 		return xerrors.Errorf("mapping node must implement yaml.Unmarshaler")

diff --git a/cli/server_test.go b/cli/server_test.go
@@ -21,7 +21,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -36,7 +35,6 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"github.com/coder/coder/cli"
-	"github.com/coder/coder/cli/clibase"
 	"github.com/coder/coder/cli/clitest"
 	"github.com/coder/coder/cli/config"
 	"github.com/coder/coder/coderd/coderdtest"
@@ -1470,46 +1468,22 @@ func TestServer(t *testing.T) {
 			client = codersdk.New(waitAccessURL(t, cfg))
 			_ = coderdtest.CreateFirstUser(t, client)
 			gotConfig, err := client.DeploymentConfig(ctx)
-			normalizeNilsInOptionSet(&wantConfig.Options)
-			normalizeNilsInOptionSet(&gotConfig.Options)
 			require.NoError(t, err, "config:\n%s\nargs: %+v", conf.String(), inv.Args)
 			gotConfig.Options.ByName("Config Path").Value.Set("")
-			require.EqualValues(t, wantConfig.Options, gotConfig.Options)
+			// We check the options individually for better error messages.
+			for i := range wantConfig.Options {
+				assert.Equal(
+					t, wantConfig.Options[i],
+					gotConfig.Options[i],
+					"option %q",
+					wantConfig.Options[i].Name,
+				)
+			}
 			w.RequireSuccess()
 		})
 	})
 }
 
-// The YAML process sets slice values to nil if they are empty, leading to
-// false negatives when comparing equality.
-func normalizeNilsInOptionSet(s *clibase.OptionSet) {
-	for i := range *s {
-		opt := &(*s)[i]
-		if opt.Value == nil {
-			continue
-		}
-
-		var (
-			v    = reflect.Indirect(reflect.ValueOf(opt.Value))
-			kind = v.Type().Kind()
-		)
-
-		if opt.YAML == "supportLinks" {
-			fmt.Printf("supportLinks: %v, %v", kind, v.IsZero())
-		}
-
-		if kind == reflect.Slice && v.Len() > 0 {
-			continue
-		}
-
-		if kind == reflect.Struct && !v.IsZero() {
-			continue
-		}
-
-		opt.Value = nil
-	}
-}
-
 func generateTLSCertificate(t testing.TB, commonName ...string) (certPath, keyPath string) {
 	dir := t.TempDir()