feat(workflow): export ErrExcludeUnknownKey for an exclude key outside the matrix
- commit
- c18027d4bf527794f69d797bd4b653218b833a25
- author
- brice · · 2026-10-13 02:33 UTC
- committer
- brice · · 2026-10-18 14:59 UTC
- parent
- 226f9bd
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +103 -5
M workflow/matrix.go +24 -5
| @@ -57,7 +57,8 @@ func (c MatrixCombo) DisplayName(base string) string { | |||
| 57 | 57 | // varying slowest; | |
| 58 | 58 | // - exclude removes every combination that contains all the key-value | |
| 59 | 59 | // pairs of one of its entries; every key of an entry must be a vector of | |
| 60 | - | // the matrix, or the matrix is an error; | |
| 60 | + | // the matrix, or the matrix is an error that matches | |
| 61 | + | // [ErrExcludeUnknownKey]; | |
| 61 | 62 | // - then each include entry is added to every remaining combination whose | |
| 62 | 63 | // values for the vector keys it mentions are equal (an entry that | |
| 63 | 64 | // mentions no vector key matches all), without overwriting the original | |
| @@ -152,10 +153,28 @@ func matrixEntries(name string, v expr.Value) ([]*expr.Dictionary, error) { | |||
| 152 | 153 | return out, nil | |
| 153 | 154 | } | |
| 154 | 155 | ||
| 155 | - | // message is an error whose text is GitHub's, which starts with a capital. | |
| 156 | - | type message string | |
| 156 | + | // ErrExcludeUnknownKey is matched, with [errors.Is], by the error that | |
| 157 | + | // [ExpandMatrix], [ParseStrategy] and [Job.EvaluateStrategy] return when an | |
| 158 | + | // entry of "exclude" names a key that is not a vector of the matrix (keys | |
| 159 | + | // that only "include" adds do not count). GitHub refuses such a workflow as | |
| 160 | + | // invalid, before any job exists, and names the run after the file path | |
| 161 | + | // rather than after the "name:" key (recorded: matrix-edge-values, | |
| 162 | + | // a-exclude-unknown-key.yml). The error carries one line per offending | |
| 163 | + | // entry; when several entries offend, errors.Is still holds. | |
| 164 | + | var ErrExcludeUnknownKey = errors.New("matrix exclude key does not match any key within the matrix") | |
| 157 | 165 | ||
| 158 | - | func (m message) Error() string { return string(m) } | |
| 166 | + | // excludeKeyError is the error for one exclude entry with an unknown key. | |
| 167 | + | type excludeKeyError struct { | |
| 168 | + | // Key is the first unknown key of the entry. | |
| 169 | + | Key string | |
| 170 | + | } | |
| 171 | + | ||
| 172 | + | // Unwrap makes errors.Is(err, [ErrExcludeUnknownKey]) hold. | |
| 173 | + | func (e *excludeKeyError) Unwrap() error { return ErrExcludeUnknownKey } | |
| 174 | + | ||
| 175 | + | func (e *excludeKeyError) Error() string { | |
| 176 | + | return fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", e.Key) | |
| 177 | + | } | |
| 159 | 178 | ||
| 160 | 179 | // errProductTooBig is returned when the cross product of the vectors is too | |
| 161 | 180 | // large to be enumerated. GitHub would count the configurations left after | |
| @@ -236,7 +255,7 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 236 | 255 | vi, ok := index[strings.ToLower(p.Key)] | |
| 237 | 256 | if !ok { | |
| 238 | 257 | // One error per offending entry, for its first unknown key. | |
| 239 | - | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | |
| 258 | + | bad = append(bad, &excludeKeyError{Key: p.Key}) | |
| 240 | 259 | unknown = true | |
| 241 | 260 | break | |
| 242 | 261 | } | |
| @@ -57,7 +57,8 @@ func (c MatrixCombo) DisplayName(base string) string { | |||||
| 57 | // varying slowest; | 57 | // varying slowest; | ||
| 58 | // - exclude removes every combination that contains all the key-value | 58 | // - exclude removes every combination that contains all the key-value | ||
| 59 | // pairs of one of its entries; every key of an entry must be a vector of | 59 | // pairs of one of its entries; every key of an entry must be a vector of | ||
| 60 | - | // the matrix, or the matrix is an error; | 60 | + | // the matrix, or the matrix is an error that matches |
| 61 | + | // [ErrExcludeUnknownKey]; | |||
| 61 | // - then each include entry is added to every remaining combination whose | 62 | // - then each include entry is added to every remaining combination whose | ||
| 62 | // values for the vector keys it mentions are equal (an entry that | 63 | // values for the vector keys it mentions are equal (an entry that | ||
| 63 | // mentions no vector key matches all), without overwriting the original | 64 | // mentions no vector key matches all), without overwriting the original | ||
| @@ -152,10 +153,28 @@ func matrixEntries(name string, v expr.Value) ([]*expr.Dictionary, error) { | |||||
| 152 | return out, nil | 153 | return out, nil | ||
| 153 | } | 154 | } | ||
| 154 | 155 | ||||
| 155 | - | // message is an error whose text is GitHub's, which starts with a capital. | 156 | + | // ErrExcludeUnknownKey is matched, with [errors.Is], by the error that |
| 156 | - | type message string | 157 | + | // [ExpandMatrix], [ParseStrategy] and [Job.EvaluateStrategy] return when an |
| 158 | + | // entry of "exclude" names a key that is not a vector of the matrix (keys | |||
| 159 | + | // that only "include" adds do not count). GitHub refuses such a workflow as | |||
| 160 | + | // invalid, before any job exists, and names the run after the file path | |||
| 161 | + | // rather than after the "name:" key (recorded: matrix-edge-values, | |||
| 162 | + | // a-exclude-unknown-key.yml). The error carries one line per offending | |||
| 163 | + | // entry; when several entries offend, errors.Is still holds. | |||
| 164 | + | var ErrExcludeUnknownKey = errors.New("matrix exclude key does not match any key within the matrix") | |||
| 157 | 165 | ||||
| 158 | - | func (m message) Error() string { return string(m) } | 166 | + | // excludeKeyError is the error for one exclude entry with an unknown key. |
| 167 | + | type excludeKeyError struct { | |||
| 168 | + | // Key is the first unknown key of the entry. | |||
| 169 | + | Key string | |||
| 170 | + | } | |||
| 171 | + | ||||
| 172 | + | // Unwrap makes errors.Is(err, [ErrExcludeUnknownKey]) hold. | |||
| 173 | + | func (e *excludeKeyError) Unwrap() error { return ErrExcludeUnknownKey } | |||
| 174 | + | ||||
| 175 | + | func (e *excludeKeyError) Error() string { | |||
| 176 | + | return fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", e.Key) | |||
| 177 | + | } | |||
| 159 | 178 | ||||
| 160 | // errProductTooBig is returned when the cross product of the vectors is too | 179 | // errProductTooBig is returned when the cross product of the vectors is too | ||
| 161 | // large to be enumerated. GitHub would count the configurations left after | 180 | // large to be enumerated. GitHub would count the configurations left after | ||
| @@ -236,7 +255,7 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 236 | vi, ok := index[strings.ToLower(p.Key)] | 255 | vi, ok := index[strings.ToLower(p.Key)] | ||
| 237 | if !ok { | 256 | if !ok { | ||
| 238 | // One error per offending entry, for its first unknown key. | 257 | // One error per offending entry, for its first unknown key. | ||
| 239 | - | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | 258 | + | bad = append(bad, &excludeKeyError{Key: p.Key}) |
| 240 | unknown = true | 259 | unknown = true | ||
| 241 | break | 260 | break | ||
| 242 | } | 261 | } | ||
M workflow/matrix_test.go +79 -0
| @@ -402,11 +402,90 @@ func TestRecordedExcludeUnknownKey(t *testing.T) { | |||
| 402 | 402 | if err == nil || err.Error() != one+"\n"+one { | |
| 403 | 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | |
| 404 | 404 | } | |
| 405 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 406 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |
| 407 | + | } | |
| 408 | + | if errors.Is(err, ErrTooManyConfigurations) { | |
| 409 | + | t.Errorf("errors.Is(%v, ErrTooManyConfigurations) = true", err) | |
| 410 | + | } | |
| 405 | 411 | // An entry that is fine does not add one. | |
| 406 | 412 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | |
| 407 | 413 | if err == nil || err.Error() != one { | |
| 408 | 414 | t.Errorf("err = %v, want %q", err, one) | |
| 409 | 415 | } | |
| 416 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 417 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |
| 418 | + | } | |
| 419 | + | } | |
| 420 | + | ||
| 421 | + | // TestExcludeUnknownKeySentinelFromJob checks that the sentinel survives | |
| 422 | + | // Job.EvaluateStrategy, which is what a server calls, on the workflow of the | |
| 423 | + | // recorded scenario (matrix-edge-values, a-exclude-unknown-key.yml). The | |
| 424 | + | // workflow parses: GitHub still refuses it, and names the run after the file. | |
| 425 | + | func TestExcludeUnknownKeySentinelFromJob(t *testing.T) { | |
| 426 | + | w, errs := Parse("a.yml", []byte(`name: a-exclude-unknown-key | |
| 427 | + | on: | |
| 428 | + | push: | |
| 429 | + | branches: ['scn/**'] | |
| 430 | + | jobs: | |
| 431 | + | main: | |
| 432 | + | runs-on: ubuntu-24.04 | |
| 433 | + | strategy: | |
| 434 | + | matrix: | |
| 435 | + | os: [one, two] | |
| 436 | + | exclude: | |
| 437 | + | - nokey: x | |
| 438 | + | - os: one | |
| 439 | + | nokey: y | |
| 440 | + | steps: | |
| 441 | + | - run: echo "os=${{ matrix.os }}" | |
| 442 | + | `)) | |
| 443 | + | if len(errs) > 0 { | |
| 444 | + | t.Fatal(errs) | |
| 445 | + | } | |
| 446 | + | _, err := w.Job("main").EvaluateStrategy(ctxbg, template.Env{}) | |
| 447 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 448 | + | t.Fatalf("EvaluateStrategy err = %v, want ErrExcludeUnknownKey", err) | |
| 449 | + | } | |
| 450 | + | // A key that only an include adds is no vector either. | |
| 451 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "include": [{"b": 1}], "exclude": [{"b": 1}]}}`)) | |
| 452 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 453 | + | t.Errorf("exclude of an include-only key: %v", err) | |
| 454 | + | } | |
| 455 | + | // Other matrix errors do not match it. | |
| 456 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "exclude": [{"a": 1}]}}`)) | |
| 457 | + | if err == nil || errors.Is(err, ErrExcludeUnknownKey) { | |
| 458 | + | t.Errorf("all excluded: %v", err) | |
| 459 | + | } | |
| 460 | + | } | |
| 461 | + | ||
| 462 | + | // TestRecordedNullAndEmptyJobIndex is matrix-edge-values b-null-values.yml, | |
| 463 | + | // v: [1, null, "", true, 1.50, "x y"]. The recording has strategy.job-index 1 | |
| 464 | + | // for matrix.v null and 2 for matrix.v "" (jobs 0..5 in the order of the | |
| 465 | + | // vector), which is what this package produces. conformance compare listed the | |
| 466 | + | // two as swapped only because both legs are named "default-name" and it pairs | |
| 467 | + | // equal names by their position in each source's job listing, which GitHub | |
| 468 | + | // does not give in matrix order. So the expansion order is the recorded one and | |
| 469 | + | // this test pins it. | |
| 470 | + | func TestRecordedNullAndEmptyJobIndex(t *testing.T) { | |
| 471 | + | s, err := ParseStrategy(ctxbg, jsonValue(t, `{"matrix": {"v": [1, null, "", true, 1.50, "x y"]}}`)) | |
| 472 | + | if err != nil { | |
| 473 | + | t.Fatal(err) | |
| 474 | + | } | |
| 475 | + | want := []string{`1`, `null`, `""`, `true`, `1.5`, `"x y"`} | |
| 476 | + | if len(s.Combos) != len(want) { | |
| 477 | + | t.Fatalf("%d combinations, want %d", len(s.Combos), len(want)) | |
| 478 | + | } | |
| 479 | + | for i, c := range s.Combos { | |
| 480 | + | v, _ := c.Values.Get("v") | |
| 481 | + | if got := expr.ToJSON(v); got != want[i] { | |
| 482 | + | t.Errorf("combination %d: matrix.v = %s, want %s", i, got, want[i]) | |
| 483 | + | } | |
| 484 | + | idx, _ := s.Context(i).Get("job-index") | |
| 485 | + | if idx != expr.Number(i) { | |
| 486 | + | t.Errorf("combination %d: strategy.job-index = %v", i, expr.ToJSON(idx)) | |
| 487 | + | } | |
| 488 | + | } | |
| 410 | 489 | } | |
| 411 | 490 | ||
| 412 | 491 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | |
| @@ -402,11 +402,90 @@ func TestRecordedExcludeUnknownKey(t *testing.T) { | |||||
| 402 | if err == nil || err.Error() != one+"\n"+one { | 402 | if err == nil || err.Error() != one+"\n"+one { | ||
| 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | ||
| 404 | } | 404 | } | ||
| 405 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 406 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |||
| 407 | + | } | |||
| 408 | + | if errors.Is(err, ErrTooManyConfigurations) { | |||
| 409 | + | t.Errorf("errors.Is(%v, ErrTooManyConfigurations) = true", err) | |||
| 410 | + | } | |||
| 405 | // An entry that is fine does not add one. | 411 | // An entry that is fine does not add one. | ||
| 406 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | 412 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | ||
| 407 | if err == nil || err.Error() != one { | 413 | if err == nil || err.Error() != one { | ||
| 408 | t.Errorf("err = %v, want %q", err, one) | 414 | t.Errorf("err = %v, want %q", err, one) | ||
| 409 | } | 415 | } | ||
| 416 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 417 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |||
| 418 | + | } | |||
| 419 | + | } | |||
| 420 | + | ||||
| 421 | + | // TestExcludeUnknownKeySentinelFromJob checks that the sentinel survives | |||
| 422 | + | // Job.EvaluateStrategy, which is what a server calls, on the workflow of the | |||
| 423 | + | // recorded scenario (matrix-edge-values, a-exclude-unknown-key.yml). The | |||
| 424 | + | // workflow parses: GitHub still refuses it, and names the run after the file. | |||
| 425 | + | func TestExcludeUnknownKeySentinelFromJob(t *testing.T) { | |||
| 426 | + | w, errs := Parse("a.yml", []byte(`name: a-exclude-unknown-key | |||
| 427 | + | on: | |||
| 428 | + | push: | |||
| 429 | + | branches: ['scn/**'] | |||
| 430 | + | jobs: | |||
| 431 | + | main: | |||
| 432 | + | runs-on: ubuntu-24.04 | |||
| 433 | + | strategy: | |||
| 434 | + | matrix: | |||
| 435 | + | os: [one, two] | |||
| 436 | + | exclude: | |||
| 437 | + | - nokey: x | |||
| 438 | + | - os: one | |||
| 439 | + | nokey: y | |||
| 440 | + | steps: | |||
| 441 | + | - run: echo "os=${{ matrix.os }}" | |||
| 442 | + | `)) | |||
| 443 | + | if len(errs) > 0 { | |||
| 444 | + | t.Fatal(errs) | |||
| 445 | + | } | |||
| 446 | + | _, err := w.Job("main").EvaluateStrategy(ctxbg, template.Env{}) | |||
| 447 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 448 | + | t.Fatalf("EvaluateStrategy err = %v, want ErrExcludeUnknownKey", err) | |||
| 449 | + | } | |||
| 450 | + | // A key that only an include adds is no vector either. | |||
| 451 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "include": [{"b": 1}], "exclude": [{"b": 1}]}}`)) | |||
| 452 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 453 | + | t.Errorf("exclude of an include-only key: %v", err) | |||
| 454 | + | } | |||
| 455 | + | // Other matrix errors do not match it. | |||
| 456 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "exclude": [{"a": 1}]}}`)) | |||
| 457 | + | if err == nil || errors.Is(err, ErrExcludeUnknownKey) { | |||
| 458 | + | t.Errorf("all excluded: %v", err) | |||
| 459 | + | } | |||
| 460 | + | } | |||
| 461 | + | ||||
| 462 | + | // TestRecordedNullAndEmptyJobIndex is matrix-edge-values b-null-values.yml, | |||
| 463 | + | // v: [1, null, "", true, 1.50, "x y"]. The recording has strategy.job-index 1 | |||
| 464 | + | // for matrix.v null and 2 for matrix.v "" (jobs 0..5 in the order of the | |||
| 465 | + | // vector), which is what this package produces. conformance compare listed the | |||
| 466 | + | // two as swapped only because both legs are named "default-name" and it pairs | |||
| 467 | + | // equal names by their position in each source's job listing, which GitHub | |||
| 468 | + | // does not give in matrix order. So the expansion order is the recorded one and | |||
| 469 | + | // this test pins it. | |||
| 470 | + | func TestRecordedNullAndEmptyJobIndex(t *testing.T) { | |||
| 471 | + | s, err := ParseStrategy(ctxbg, jsonValue(t, `{"matrix": {"v": [1, null, "", true, 1.50, "x y"]}}`)) | |||
| 472 | + | if err != nil { | |||
| 473 | + | t.Fatal(err) | |||
| 474 | + | } | |||
| 475 | + | want := []string{`1`, `null`, `""`, `true`, `1.5`, `"x y"`} | |||
| 476 | + | if len(s.Combos) != len(want) { | |||
| 477 | + | t.Fatalf("%d combinations, want %d", len(s.Combos), len(want)) | |||
| 478 | + | } | |||
| 479 | + | for i, c := range s.Combos { | |||
| 480 | + | v, _ := c.Values.Get("v") | |||
| 481 | + | if got := expr.ToJSON(v); got != want[i] { | |||
| 482 | + | t.Errorf("combination %d: matrix.v = %s, want %s", i, got, want[i]) | |||
| 483 | + | } | |||
| 484 | + | idx, _ := s.Context(i).Get("job-index") | |||
| 485 | + | if idx != expr.Number(i) { | |||
| 486 | + | t.Errorf("combination %d: strategy.job-index = %v", i, expr.ToJSON(idx)) | |||
| 487 | + | } | |||
| 488 | + | } | |||
| 410 | } | 489 | } | ||
| 411 | 490 | ||||
| 412 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | 491 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | ||