Verify status fields before uploading PullRequest

The pullrequest pipeline resource now verifies the statuses before
uploading them. As specified in #1818, this is to improve the
debuggability of pullrequest resources, by preventing a silent failure
e.g. if there has been a change in the fields' names of a status object.

Signed-off-by: Arash Deshmeh <[email protected]>

pkg/pullrequest/api.go

@@ -259,12 +259,29 @@ func (h *Handler) createNewComments(ctx context.Context, comments []*scm.Comment
 	return merr
 }
 
+func validateStatuses(statuses []*scm.Status) error {
+	var merr error
+	for _, s := range statuses {
+		if s.Label == "" {
+			merr = multierror.Append(merr, fmt.Errorf("invalid status: \"Label\" should not be empty: %v", *s))
+		}
+		if s.State.String() == scm.StateUnknown.String() {
+			merr = multierror.Append(merr, fmt.Errorf("invalid status: \"State\" is empty or has invalid value: %v", *s))
+		}
+	}
+	return merr
+}
+
 func (h *Handler) uploadStatuses(ctx context.Context, statuses []*scm.Status, sha string) error {
 	if statuses == nil {
 		h.logger.Info("Skipping statuses, nothing to set.")
 		return nil
 	}
 
+	if err := validateStatuses(statuses); err != nil {
+		return err
+	}
+
 	h.logger.Infof("Looking for existing status on %s", sha)
 	cs, _, err := h.client.Repositories.ListStatus(ctx, h.repo, sha, scm.ListOptions{})
 	if err != nil {

pkg/pullrequest/api_test.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/go-scm/scm/driver/fake"
 
@@ -268,6 +269,46 @@ func TestUpload_UpdateStatus(t *testing.T) {
 	}
 }
 
+func TestUpload_Invalid_Status(t *testing.T) {
+	ctx := context.Background()
+	h, _ := newHandler(t)
+
+	r := defaultResource()
+	r.Statuses = []*scm.Status{
+		{
+			Label:  "Tekton",
+			Desc:   "Status with empty State field",
+			Target: "https://tekton.dev",
+		},
+		{
+			State:  scm.StateSuccess,
+			Desc:   "Status without label",
+			Target: "https://tekton.dev",
+		},
+	}
+
+	expectedErrors := []string{
+		"invalid status: \"State\" is empty or has invalid value: {unknown Tekton Status with empty State field https://tekton.dev}",
+		"invalid status: \"Label\" should not be empty: {success  Status without label https://tekton.dev}",
+	}
+	err := h.Upload(ctx, r)
+	if err == nil {
+		t.Fatal("expected errors, got nil")
+	}
+	merr, ok := err.(*multierror.Error)
+	if !ok {
+		t.Fatalf("expected error of type multierror, got %#v", merr)
+	}
+	if len(merr.Errors) != 2 {
+		t.Fatalf("expected 2 errors, got %d", len(merr.Errors))
+	}
+	for i, err := range merr.Errors {
+		if d := cmp.Diff(expectedErrors[i], err.Error()); d != "" {
+			t.Errorf("Upload status error diff -want, +got: %v", d)
+		}
+	}
+}
+
 func TestUpload_NewLabel(t *testing.T) {
 	ctx := context.Background()
 	h, _ := newHandler(t)