From 8213ed3a3a386619ceaaf57edd5d9c7c29447db9 Mon Sep 17 00:00:00 2001
From: Stephen Muth <smuth4@gmail.com>
Date: Sat, 31 Dec 2022 14:22:00 -0500
Subject: [PATCH 1/2] Rework status constraint logic for successes

Since "success" and "failure" are the only two possible values, and
"success" is considered to be included by default, the existing code
can also be simplified a little.

Fixes #1181
---
 pipeline/frontend/yaml/compiler/convert.go    |  7 +++----
 .../frontend/yaml/constraint/constraint.go    | 19 ++++++++++++-------
 .../yaml/constraint/constraint_test.go        | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go
index 5ff01b99e4..b555bfcf38 100644
--- a/pipeline/frontend/yaml/compiler/convert.go
+++ b/pipeline/frontend/yaml/compiler/convert.go
@@ -134,11 +134,10 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section
 		cpuSet = c.reslimit.CPUSet
 	}
 
-	// all constraints must exclude success.
-	onSuccess := container.When.IsEmpty() ||
-		!container.When.ExcludesStatus("success")
+	// all constraints must not include success.
+	onSuccess := container.When.IncludesStatusSuccess()
 	// at least one constraint must include the status failure.
-	onFailure := container.When.IncludesStatus("failure")
+	onFailure := container.When.IncludesStatusFailure()
 
 	failure := container.Failure
 	if container.Failure == "" {
diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go
index 2557e0e13a..6db26f69b2 100644
--- a/pipeline/frontend/yaml/constraint/constraint.go
+++ b/pipeline/frontend/yaml/constraint/constraint.go
@@ -79,9 +79,9 @@ func (when *When) Match(metadata frontend.Metadata, global bool) (bool, error) {
 	return false, nil
 }
 
-func (when *When) IncludesStatus(status string) bool {
+func (when *When) IncludesStatusFailure() bool {
 	for _, c := range when.Constraints {
-		if c.Status.Includes(status) {
+		if c.Status.Includes("failure") {
 			return true
 		}
 	}
@@ -89,14 +89,19 @@ func (when *When) IncludesStatus(status string) bool {
 	return false
 }
 
-func (when *When) ExcludesStatus(status string) bool {
+func (when *When) IncludesStatusSuccess() bool {
+	// "success" acts differently than "failure" in that it's
+	// presumed to be included unless it's specifically not part
+	// of the list
+	if when.IsEmpty() {
+		return true
+	}
 	for _, c := range when.Constraints {
-		if !c.Status.Excludes(status) {
-			return false
+		if len(c.Status.Include) == 0 || c.Status.Includes("success") {
+			return true
 		}
 	}
-
-	return len(when.Constraints) > 0
+	return false
 }
 
 // False if (any) non local
diff --git a/pipeline/frontend/yaml/constraint/constraint_test.go b/pipeline/frontend/yaml/constraint/constraint_test.go
index f5fec49be1..3c72f59b04 100644
--- a/pipeline/frontend/yaml/constraint/constraint_test.go
+++ b/pipeline/frontend/yaml/constraint/constraint_test.go
@@ -381,6 +381,24 @@ func TestConstraintMap(t *testing.T) {
 	}
 }
 
+func TestConstraintStatusSuccess(t *testing.T) {
+	testdata := []struct {
+		conf string
+		want bool
+	}{
+		{conf: "", want: true},
+		{conf: "{status: [failure]}", want: false},
+		{conf: "{status: [success]}", want: true},
+		{conf: "{status: [failure, success]}", want: true},
+		{conf: "{status: {exclude: [success], include: [failure]}}", want: false},
+		{conf: "{status: {exclude: [failure], include: [success]}}", want: true},
+	}
+	for _, test := range testdata {
+		c := parseConstraints(t, test.conf)
+		assert.Equal(t, test.want, c.IncludesStatusSuccess(), "when: '%s'", test.conf)
+	}
+}
+
 func TestConstraints(t *testing.T) {
 	testdata := []struct {
 		desc string

From 3284a4d704912e03cd00e2ca8519875e939687d5 Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Mon, 2 Jan 2023 05:38:31 +0100
Subject: [PATCH 2/2] Update pipeline/frontend/yaml/compiler/convert.go

---
 pipeline/frontend/yaml/compiler/convert.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go
index b555bfcf38..37d899e014 100644
--- a/pipeline/frontend/yaml/compiler/convert.go
+++ b/pipeline/frontend/yaml/compiler/convert.go
@@ -134,7 +134,7 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section
 		cpuSet = c.reslimit.CPUSet
 	}
 
-	// all constraints must not include success.
+	// at least one constraint contain status success, or all constraints have no status set
 	onSuccess := container.When.IncludesStatusSuccess()
 	// at least one constraint must include the status failure.
 	onFailure := container.When.IncludesStatusFailure()