Merge pull request #31 from buildkite/ps-46-silent-parse-failures

Warnings fix and improvements

ordered/unmarshal.go

@@ -169,9 +169,9 @@ func Unmarshal(src, dst any) error {
 				x := reflect.New(etype) // *E
 				err := Unmarshal(a, x.Interface())
 				if w := warning.As(err); w != nil {
-					warns = append(warns, w.Wrapf("while unmarshaling item %d of %d", i, len(tsrc)))
+					warns = append(warns, w.Wrapf("while unmarshaling item at index %d of %d", i, len(tsrc)))
 				} else if err != nil {
-					return err
+					return fmt.Errorf("unmarshaling item at index %d of %d: %w", i, len(tsrc), err)
 				}
 				sdst = reflect.Append(sdst, x.Elem())
 			}
@@ -282,7 +282,7 @@ func (m *Map[K, V]) decodeInto(target any) error {
 			if w := warning.As(err); w != nil {
 				warns = append(warns, w.Wrapf("while unmarshaling value for key %q", k))
 			} else if err != nil {
-				return err
+				return fmt.Errorf("unmarshaling value for key %q: %w", k, err)
 			}
 
 			innerValue.SetMapIndex(reflect.ValueOf(k), nv.Elem())
@@ -431,7 +431,7 @@ func (m *Map[K, V]) UnmarshalOrdered(src any) error {
 		if w := warning.As(err); w != nil {
 			warns = append(warns, w.Wrapf("while unmarshaling the value for key %q", k))
 		} else if err != nil {
-			return err
+			return fmt.Errorf("unmarshaling value for key %q: %w", k, err)
 		}
 		tm.Set(k, dv)
 		return nil

parser_test.go

@@ -1097,6 +1097,65 @@ steps:
 	}
 }
 
+func TestParserEmitsWarningWithTopLevelStepSequence(t *testing.T) {
+	input := strings.NewReader(`---
+  - catawumpus
+`)
+	got, err := Parse(input)
+	if !warning.Is(err) {
+		t.Fatalf("Parse(input) error = %v, want a warning", err)
+	}
+
+	errs := warning.As(err).Unwrap()
+	wantErrs := []error{
+		ErrUnknownStepType,
+	}
+	errorComparer := cmp.Comparer(func(x, y error) bool {
+		return errors.Is(x, y) || errors.Is(y, x)
+	})
+	if diff := cmp.Diff(errs, wantErrs, errorComparer); diff != "" {
+		t.Errorf("underlying errors diff (-got +want):\n%s", diff)
+		t.Logf("Full parse warnings:\n%v", err)
+	}
+
+	want := &Pipeline{
+		Steps: Steps{
+			&UnknownStep{
+				Contents: "catawumpus",
+			},
+		},
+	}
+
+	if diff := cmp.Diff(got, want, cmp.Comparer(ordered.EqualSA)); diff != "" {
+		t.Errorf("parsed pipeline diff (-got +want):\n%s", diff)
+	}
+
+	gotJSON, err := json.MarshalIndent(got, "", "  ")
+	if err != nil {
+		t.Errorf(`json.MarshalIndent(got, "", "  ") error = %v`, err)
+	}
+	const wantJSON = `{
+  "steps": [
+    "catawumpus"
+  ]
+}`
+	if diff := cmp.Diff(string(gotJSON), wantJSON); diff != "" {
+		t.Errorf("marshalled JSON diff (-got +want):\n%s", diff)
+	}
+
+	gotYAML, err := yaml.Marshal(got)
+	if err != nil {
+		t.Errorf("yaml.Marshal(got) error = %v", err)
+	}
+
+	wantYAML := `steps:
+    - catawumpus
+`
+	if diff := cmp.Diff(string(gotYAML), wantYAML); diff != "" {
+		t.Errorf("marshalled YAML diff (-got +want):\n%s", diff)
+	}
+}
+
 func TestParserParsesEnvAndStepsNull(t *testing.T) {
 	input := strings.NewReader(`---
 env: null

pipeline.go

@@ -48,7 +48,9 @@ func (p *Pipeline) UnmarshalOrdered(o any) error {
 	case []any:
 		// A pipeline can be a sequence of steps.
 		err := ordered.Unmarshal(o, &p.Steps)
-		if err != nil {
+		if w := warning.As(err); w != nil {
+			warns = append(warns, w)
+		} else if err != nil {
 			return fmt.Errorf("unmarshaling steps: %w", err)
 		}