fix(executor): give a job without a matrix a null matrix context
- commit
- 40f1da331b4ef279dca5e9683af097bff10d8790
- author
- brice · · 2026-10-12 22:47 UTC
- parent
- f9d254a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +39 -4
M internal/executor/contexts.go +8 -0
| @@ -257,6 +257,14 @@ func (s *State) initRest(root *expr.Dictionary, matrixJSON string, r RunnerInfo) | |||
| 257 | 257 | return fmt.Errorf("job matrix: %w", err) | |
| 258 | 258 | } | |
| 259 | 259 | } | |
| 260 | + | // A job without a matrix has the matrix context null on github.com, not an | |
| 261 | + | // empty object (recordings: "matrix": null in the 176 of 221 jobs that have | |
| 262 | + | // none, and toJSON(matrix) prints null). The server sends "{}" for such a | |
| 263 | + | // job (runner.proto: Job.matrix), so an empty matrix becomes null here. | |
| 264 | + | // matrix.x still works: a property of null is null, as for a missing key. | |
| 265 | + | if d, ok := s.other["matrix"].(*expr.Dictionary); !ok || d.Len() == 0 { | |
| 266 | + | s.other["matrix"] = expr.Null{} | |
| 267 | + | } | |
| 260 | 268 | // A context the job does not have is an empty object, like the server's | |
| 261 | 269 | // ("needs": {}, "inputs": {}, "vars": {}); matrix and strategy are null | |
| 262 | 270 | // when absent (recordings: "matrix": null). | |
| @@ -257,6 +257,14 @@ func (s *State) initRest(root *expr.Dictionary, matrixJSON string, r RunnerInfo) | |||||
| 257 | return fmt.Errorf("job matrix: %w", err) | 257 | return fmt.Errorf("job matrix: %w", err) | ||
| 258 | } | 258 | } | ||
| 259 | } | 259 | } | ||
| 260 | + | // A job without a matrix has the matrix context null on github.com, not an | |||
| 261 | + | // empty object (recordings: "matrix": null in the 176 of 221 jobs that have | |||
| 262 | + | // none, and toJSON(matrix) prints null). The server sends "{}" for such a | |||
| 263 | + | // job (runner.proto: Job.matrix), so an empty matrix becomes null here. | |||
| 264 | + | // matrix.x still works: a property of null is null, as for a missing key. | |||
| 265 | + | if d, ok := s.other["matrix"].(*expr.Dictionary); !ok || d.Len() == 0 { | |||
| 266 | + | s.other["matrix"] = expr.Null{} | |||
| 267 | + | } | |||
| 260 | // A context the job does not have is an empty object, like the server's | 268 | // A context the job does not have is an empty object, like the server's | ||
| 261 | // ("needs": {}, "inputs": {}, "vars": {}); matrix and strategy are null | 269 | // ("needs": {}, "inputs": {}, "vars": {}); matrix and strategy are null | ||
| 262 | // when absent (recordings: "matrix": null). | 270 | // when absent (recordings: "matrix": null). | ||
M internal/executor/contexts_test.go +12 -4
| @@ -102,15 +102,23 @@ func TestServerContextsAndDefaults(t *testing.T) { | |||
| 102 | 102 | t.Errorf("context %s missing", k) | |
| 103 | 103 | } | |
| 104 | 104 | } | |
| 105 | - | // Absent contexts: needs/inputs/vars are empty objects, matrix is not set | |
| 106 | - | // (null in expressions), as the recordings show. | |
| 105 | + | // Absent contexts: needs/inputs/vars are empty objects, matrix is null | |
| 106 | + | // (also when the server sends {}), as the recordings show. | |
| 107 | 107 | s2, _, err := NewState(`{"github":{"repository":"o/r"}}`, "{}", "j", RunnerInfo{}, "") | |
| 108 | 108 | if err != nil { | |
| 109 | 109 | t.Fatal(err) | |
| 110 | 110 | } | |
| 111 | 111 | v2 := s2.Values(nil) | |
| 112 | - | if _, ok := v2["matrix"]; ok { | |
| 113 | - | t.Error("matrix must be absent without a matrix") | |
| 112 | + | if _, ok := v2["matrix"].(expr.Null); !ok { | |
| 113 | + | t.Errorf("matrix without a matrix: %v, want null", v2["matrix"]) | |
| 114 | + | } | |
| 115 | + | // The server's "{}" in the contexts is null too. | |
| 116 | + | s3, _, err := NewState(`{"github":{"repository":"o/r"},"matrix":{}}`, "{}", "j", RunnerInfo{}, "") | |
| 117 | + | if err != nil { | |
| 118 | + | t.Fatal(err) | |
| 119 | + | } | |
| 120 | + | if _, ok := s3.Values(nil)["matrix"].(expr.Null); !ok { | |
| 121 | + | t.Errorf("empty matrix: %v, want null", s3.Values(nil)["matrix"]) | |
| 114 | 122 | } | |
| 115 | 123 | if d, ok := v2["needs"].(*expr.Dictionary); !ok || d.Len() != 0 { | |
| 116 | 124 | t.Errorf("needs %v", v2["needs"]) | |
| @@ -102,15 +102,23 @@ func TestServerContextsAndDefaults(t *testing.T) { | |||||
| 102 | t.Errorf("context %s missing", k) | 102 | t.Errorf("context %s missing", k) | ||
| 103 | } | 103 | } | ||
| 104 | } | 104 | } | ||
| 105 | - | // Absent contexts: needs/inputs/vars are empty objects, matrix is not set | 105 | + | // Absent contexts: needs/inputs/vars are empty objects, matrix is null |
| 106 | - | // (null in expressions), as the recordings show. | 106 | + | // (also when the server sends {}), as the recordings show. |
| 107 | s2, _, err := NewState(`{"github":{"repository":"o/r"}}`, "{}", "j", RunnerInfo{}, "") | 107 | s2, _, err := NewState(`{"github":{"repository":"o/r"}}`, "{}", "j", RunnerInfo{}, "") | ||
| 108 | if err != nil { | 108 | if err != nil { | ||
| 109 | t.Fatal(err) | 109 | t.Fatal(err) | ||
| 110 | } | 110 | } | ||
| 111 | v2 := s2.Values(nil) | 111 | v2 := s2.Values(nil) | ||
| 112 | - | if _, ok := v2["matrix"]; ok { | 112 | + | if _, ok := v2["matrix"].(expr.Null); !ok { |
| 113 | - | t.Error("matrix must be absent without a matrix") | 113 | + | t.Errorf("matrix without a matrix: %v, want null", v2["matrix"]) |
| 114 | + | } | |||
| 115 | + | // The server's "{}" in the contexts is null too. | |||
| 116 | + | s3, _, err := NewState(`{"github":{"repository":"o/r"},"matrix":{}}`, "{}", "j", RunnerInfo{}, "") | |||
| 117 | + | if err != nil { | |||
| 118 | + | t.Fatal(err) | |||
| 119 | + | } | |||
| 120 | + | if _, ok := s3.Values(nil)["matrix"].(expr.Null); !ok { | |||
| 121 | + | t.Errorf("empty matrix: %v, want null", s3.Values(nil)["matrix"]) | |||
| 114 | } | 122 | } | ||
| 115 | if d, ok := v2["needs"].(*expr.Dictionary); !ok || d.Len() != 0 { | 123 | if d, ok := v2["needs"].(*expr.Dictionary); !ok || d.Len() != 0 { | ||
| 116 | t.Errorf("needs %v", v2["needs"]) | 124 | t.Errorf("needs %v", v2["needs"]) | ||
M internal/executor/job_test.go +19 -0
| @@ -645,3 +645,22 @@ func TestGithubActionInStepEnvIsTheStepsOwnID(t *testing.T) { | |||
| 645 | 645 | t.Errorf("third step: %q", got) | |
| 646 | 646 | } | |
| 647 | 647 | } | |
| 648 | + | ||
| 649 | + | // Without a matrix the matrix context is null, as on github.com (toJSON prints | |
| 650 | + | // null), and matrix.x is still readable (null, so an empty string). | |
| 651 | + | func TestMatrixContextWithoutMatrixIsNull(t *testing.T) { | |
| 652 | + | needTools(t, "bash") | |
| 653 | + | rec, res := runLocal(t, wfHead+` steps: | |
| 654 | + | - run: echo "json=$J x=[$X] eq=$E" | |
| 655 | + | env: | |
| 656 | + | J: ${{ toJSON(matrix) }} | |
| 657 | + | X: ${{ matrix.x }} | |
| 658 | + | E: ${{ matrix == null }} | |
| 659 | + | `, "j", nil) | |
| 660 | + | if res.Conclusion != ConclusionSuccess { | |
| 661 | + | t.Fatalf("%+v %v", res, logOf(rec.steps[1])) | |
| 662 | + | } | |
| 663 | + | if got := rec.steps[1].Log[len(rec.steps[1].Log)-1]; got != "json=null x=[] eq=true" { | |
| 664 | + | t.Errorf("%q", got) | |
| 665 | + | } | |
| 666 | + | } | |
| @@ -645,3 +645,22 @@ func TestGithubActionInStepEnvIsTheStepsOwnID(t *testing.T) { | |||||
| 645 | t.Errorf("third step: %q", got) | 645 | t.Errorf("third step: %q", got) | ||
| 646 | } | 646 | } | ||
| 647 | } | 647 | } | ||
| 648 | + | ||||
| 649 | + | // Without a matrix the matrix context is null, as on github.com (toJSON prints | |||
| 650 | + | // null), and matrix.x is still readable (null, so an empty string). | |||
| 651 | + | func TestMatrixContextWithoutMatrixIsNull(t *testing.T) { | |||
| 652 | + | needTools(t, "bash") | |||
| 653 | + | rec, res := runLocal(t, wfHead+` steps: | |||
| 654 | + | - run: echo "json=$J x=[$X] eq=$E" | |||
| 655 | + | env: | |||
| 656 | + | J: ${{ toJSON(matrix) }} | |||
| 657 | + | X: ${{ matrix.x }} | |||
| 658 | + | E: ${{ matrix == null }} | |||
| 659 | + | `, "j", nil) | |||
| 660 | + | if res.Conclusion != ConclusionSuccess { | |||
| 661 | + | t.Fatalf("%+v %v", res, logOf(rec.steps[1])) | |||
| 662 | + | } | |||
| 663 | + | if got := rec.steps[1].Log[len(rec.steps[1].Log)-1]; got != "json=null x=[] eq=true" { | |||
| 664 | + | t.Errorf("%q", got) | |||
| 665 | + | } | |||
| 666 | + | } | |||