fix: processing of strategy.matrix.include (#1200)

* Update workflow.go * Update workflow.go * Update workflow.go * Update workflow.go * Update workflow.go * Update workflow.go * Add Tests * Update workflow.go * Modify Test * use tabs Co-authored-by: Casey Lee <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
nektos · Jun 20, 2022 · c30bc82 · c30bc82
1 parent c3fb686
commit c30bc82
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 1 deletion.
diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go
@@ -224,26 +224,37 @@ func (j *Job) GetMatrixes() []map[string]interface{} {
 
 		if m := j.Matrix(); m != nil {
 			includes := make([]map[string]interface{}, 0)
+			extraIncludes := make([]map[string]interface{}, 0)
 			for _, v := range m["include"] {
 				switch t := v.(type) {
 				case []interface{}:
 					for _, i := range t {
 						i := i.(map[string]interface{})
+						extraInclude := true
 						for k := range i {
 							if _, ok := m[k]; ok {
 								includes = append(includes, i)
+								extraInclude = false
 								break
 							}
 						}
+						if extraInclude {
+							extraIncludes = append(extraIncludes, i)
+						}
 					}
 				case interface{}:
 					v := v.(map[string]interface{})
+					extraInclude := true
 					for k := range v {
 						if _, ok := m[k]; ok {
 							includes = append(includes, v)
+							extraInclude = false
 							break
 						}
 					}
+					if extraInclude {
+						extraIncludes = append(extraIncludes, v)
+					}
 				}
 			}
 			delete(m, "include")
@@ -274,9 +285,27 @@ func (j *Job) GetMatrixes() []map[string]interface{} {
 				matrixes = append(matrixes, matrix)
 			}
 			for _, include := range includes {
+				matched := false
+				for _, matrix := range matrixes {
+					if commonKeysMatch2(matrix, include, m) {
+						matched = true
+						log.Debugf("Adding include values '%v' to existing entry", include)
+						for k, v := range include {
+							matrix[k] = v
+						}
+					}
+				}
+				if !matched {
+					extraIncludes = append(extraIncludes, include)
+				}
+			}
+			for _, include := range extraIncludes {
 				log.Debugf("Adding include '%v'", include)
 				matrixes = append(matrixes, include)
 			}
+			if len(matrixes) == 0 {
+				matrixes = append(matrixes, make(map[string]interface{}))
+			}
 		} else {
 			matrixes = append(matrixes, make(map[string]interface{}))
 		}
@@ -295,6 +324,16 @@ func commonKeysMatch(a map[string]interface{}, b map[string]interface{}) bool {
 	return true
 }
 
+func commonKeysMatch2(a map[string]interface{}, b map[string]interface{}, m map[string][]interface{}) bool {
+	for aKey, aVal := range a {
+		_, useKey := m[aKey]
+		if bVal, ok := b[aKey]; useKey && ok && !reflect.DeepEqual(aVal, bVal) {
+			return false
+		}
+	}
+	return true
+}
+
 // ContainerSpec is the specification of the container to use for the job
 type ContainerSpec struct {
 	Image       string            `yaml:"image"`

diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go
@@ -247,6 +247,7 @@ func TestReadWorkflow_Strategy(t *testing.T) {
 			{"datacenter": "site-c", "node-version": "14.x", "site": "staging"},
 			{"datacenter": "site-c", "node-version": "16.x", "site": "staging"},
 			{"datacenter": "site-d", "node-version": "16.x", "site": "staging"},
+			{"php-version": 5.4},
 			{"datacenter": "site-a", "node-version": "10.x", "site": "prod"},
 			{"datacenter": "site-b", "node-version": "12.x", "site": "dev"},
 		},

diff --git a/pkg/runner/testdata/evalmatrix/push.yml b/pkg/runner/testdata/evalmatrix/push.yml
@@ -15,4 +15,65 @@ jobs:
         echo $MATRIX
         exit ${{matrix.A && '0' || '1'}}
       env:
-        MATRIX: ${{toJSON(matrix)}}
+        MATRIX: ${{toJSON(matrix)}}
+  _additionalInclude_0:
+    strategy:
+      matrix:
+        include:
+        - def: val
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check if the matrix key A exists
+      run: |
+        echo $MATRIX
+        exit ${{matrix.def == 'val' && '0' || '1'}}
+      env:
+        MATRIX: ${{toJSON(matrix)}}
+    - run: |
+        echo "::set-output name=result::success"
+      id: result
+    outputs:
+      result: ${{ steps.result.outputs.result }}
+  _additionalInclude_1:
+    needs: _additionalInclude_0
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check if the matrix key A exists
+      run: |
+        echo $MATRIX
+        exit ${{needs._additionalInclude_0.outputs.result == 'success' && '0' || '1'}}
+  _additionalProperties_0:
+    strategy:
+      matrix:
+        x:
+        - 0
+        y:
+        - 0
+        z:
+        - 0
+        include:
+        - def: val
+          z: 0
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check if the matrix key A exists
+      run: |
+        echo $MATRIX
+        exit ${{matrix.def == 'val' && matrix.x == 0 && matrix.y == 0 && matrix.z == 0 && '0' || '1'}}
+      env:
+        MATRIX: ${{toJSON(matrix)}}
+    - run: |
+        echo "::set-output name=result::success"
+      id: result
+    outputs:
+      result: ${{ steps.result.outputs.result }}
+  _additionalProperties_1:
+    needs: _additionalProperties_0
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check if the matrix key A exists
+      run: |
+        echo $MATRIX
+        exit ${{needs._additionalProperties_0.outputs.result == 'success' && '0' || '1'}}