Merge pull request 'fix(workflow): align push filters, skip directives and matrix rules with github.com recordings' (#4) from fix/recorded-rules into main
11 files · +940 -168
M workflow/PORTING.md +53 -15
| @@ -40,23 +40,58 @@ code. | |||
| 40 | 40 | (`success() && (cond)` when the condition calls none of `success`, | |
| 41 | 41 | `failure`, `cancelled`, `always`, looking through nested calls). | |
| 42 | 42 | ||
| 43 | + | ## Recorded (github.com, ermite-conformance corpus) | |
| 44 | + | ||
| 45 | + | Opt-in check against the corpus itself: `ERMITE_CORPUS=<ermite-conformance | |
| 46 | + | checkout> go test ./workflow -run Corpus` (`corpus_test.go`) replays the | |
| 47 | + | scenario files and compares the runs SkipCI, MatchPush and Parse predict with | |
| 48 | + | the recordings. The cases copied into the unit tests stay. | |
| 49 | + | ||
| 50 | + | - Status functions (`success()`, `failure()`, `always()`, `cancelled()`) in | |
| 51 | + | `env` or `run` expressions make the workflow invalid (recorded: | |
| 52 | + | expr-status-in-env, expr-status-in-run, each a failed run with no job). The | |
| 53 | + | text `Unrecognized function: 'success'. Located at position 1 within | |
| 54 | + | expression: success()` is the one observed on the github.com run page, not in | |
| 55 | + | the recordings; `TestRecordedInvalidWorkflows`. | |
| 56 | + | - Invalid workflow messages (the recordings hold the failed run only, not the | |
| 57 | + | text; the messages are those of the reference parser, which refuses the same | |
| 58 | + | files): `The workflow must contain at least one job with | |
| 59 | + | no dependencies.` (no root job: needs on an unknown job or a cycle with no | |
| 60 | + | other root), `Job 'x' depends on unknown job 'y'.`, `Job 'x' depends on job | |
| 61 | + | 'y' which creates a cycle in the dependency graph.`, `Required property is | |
| 62 | + | missing: runs-on`, `Unexpected value 'unknown-key'` (invalid-needs, | |
| 63 | + | invalid-schema). | |
| 64 | + | ||
| 43 | 65 | ## To verify (recordings) | |
| 44 | 66 | ||
| 45 | 67 | - [ ] **Glob syntax** (`glob.go`): `*`, `**`, `?`, `+`, `[]`, `\`, `!`. The | |
| 46 | 68 | cheat sheet's examples pass; `**/` matching nothing, `?` and `+` after a | |
| 47 | 69 | `]`, `*` or an escaped character, `[!x]`, an unclosed `[` (literal here) | |
| 48 | 70 | and `/`-prefixed patterns are assumed. | |
| 49 | - | - [ ] **`MatchPush`**: deletes never run; more than 3 tags run nothing for | |
| 50 | - | tags (not for the branch); branches-only filter and a tag push (no run) | |
| 51 | - | and the reverse; with both filters each ref uses its own; `paths` only | |
| 52 | - | for branches; first 300 files (also for `paths-ignore`); truncated list | |
| 53 | - | passes; a push whose diff is empty never matches a `paths` filter and | |
| 54 | - | always matches a `paths-ignore` one; unknown ref kinds never match. | |
| 71 | + | - [x] **`MatchPush`** (recorded: push-tags-limit, filter-tags, | |
| 72 | + | filter-branches-ignore, filter-tag-branches-only, filter-paths-tag, | |
| 73 | + | filter-paths*, push-delete-*): deletes never run; more than 3 tags run | |
| 74 | + | nothing for tags (not for the branch); branches-only filter and a tag | |
| 75 | + | push (no run) and the reverse; with both filters each ref uses its own; | |
| 76 | + | `paths` only for branches; the first **3000** files, also for | |
| 77 | + | `paths-ignore` (not 300). Still assumed: a push whose diff is empty | |
| 78 | + | never matches a `paths` filter and always matches a `paths-ignore` one; | |
| 79 | + | unknown ref kinds never match; truncated list passes. | |
| 55 | 80 | - [ ] **`PushInfo.Created`** is not used: a new branch's file list is the | |
| 56 | 81 | caller's job (diff against the default branch merge-base). | |
| 57 | - | - [ ] **`SkipCI`**: marker case, trailer rules (see its doc comment); whether | |
| 58 | - | only the head commit counts; `[skip ci]` in the subject vs the body. | |
| 59 | - | - [ ] **`ExpandMatrix`**: order of the product (first key slowest), include | |
| 82 | + | - [x] **`SkipCI`** (recorded: skip-directives, skip-directives-placement, | |
| 83 | + | skip-checks-trailer, skip-directive-tag): markers are case-sensitive, | |
| 84 | + | anywhere; `skip-checks: true` needs two blank lines before it and | |
| 85 | + | nothing after it, in exact case. The tag case is the caller's: the | |
| 86 | + | message of the commit the tag points at. Assumed, not recorded: only | |
| 87 | + | the head commit counts (the scenarios push one commit at a time); a | |
| 88 | + | line of spaces or tabs is not a blank line; more than two blank lines | |
| 89 | + | count like two. | |
| 90 | + | - [ ] **`ExpandMatrix`**: the unknown exclude key error (once per offending | |
| 91 | + | entry) and the over-256 message (`configurationLimitError`) are the | |
| 92 | + | texts observed on the github.com run page for matrix-edge-values and | |
| 93 | + | matrix-over-256, not in the recordings (a failed run with no job). To | |
| 94 | + | verify: order of the product (first key slowest), include | |
| 60 | 95 | semantics (added to every matching combination, never overwriting | |
| 61 | 96 | original values, later includes overwrite earlier added values, a | |
| 62 | 97 | non-matching include is a new combination, match by vector keys only), | |
| @@ -64,18 +99,21 @@ code. | |||
| 64 | 99 | combinations after include, key case-insensitivity, value comparison | |
| 65 | 100 | (same kind; objects by containment), the error when all combinations are | |
| 66 | 101 | excluded (GitHub may fail the run differently), empty vector error. | |
| 67 | - | - [ ] A **null matrix value** is shown as an empty segment (`job (, 1)`); | |
| 68 | - | GitHub may skip it. | |
| 102 | + | - [x] A **null or empty (`''`) matrix value** is not shown in the job name | |
| 103 | + | (recorded: matrix-edge-values). | |
| 69 | 104 | - [ ] **Job display names** (`MatrixCombo.DisplayName`, `Job.DisplayName`): | |
| 70 | - | `name (v1, v2)` listing every scalar value of the combination in key | |
| 71 | - | order, then keys added by include; a name that reads the `matrix` | |
| 105 | + | recorded in matrix-job-name and matrix-edge-values: `name (v1, v2)` | |
| 106 | + | listing the values in key order, numbers and booleans as | |
| 107 | + | `CoerceString` shows them, a name that does not read `matrix` followed | |
| 108 | + | by the values. Assumed: the keys added by include come after the vector | |
| 109 | + | keys; arrays and objects are left out; a name that reads the `matrix` | |
| 72 | 110 | context (found in the expression's syntax tree, not by searching the | |
| 73 | - | text) is used as is. Booleans/numbers/null are shown with `CoerceString`. | |
| 111 | + | text) is used as is. | |
| 74 | 112 | - [ ] **`strategy` context** (`Strategy.Context`): `fail-fast`, `job-index`, | |
| 75 | 113 | `job-total`, `max-parallel` (the job count when unlimited). | |
| 76 | 114 | - [ ] `fail-fast` and `max-parallel` accept strings (`"false"`, `"2"`) after | |
| 77 | 115 | evaluation; `max-parallel` must be a whole number ≥ 1. | |
| 78 | - | - [ ] The 1 000-commit and 300-file limits themselves are the caller's. | |
| 116 | + | - [ ] The 1 000-commit and 3 000-file limits themselves are the caller's. | |
| 79 | 117 | ||
| 80 | 118 | ## New in the second pass | |
| 81 | 119 | ||
| @@ -40,23 +40,58 @@ code. | |||||
| 40 | (`success() && (cond)` when the condition calls none of `success`, | 40 | (`success() && (cond)` when the condition calls none of `success`, | ||
| 41 | `failure`, `cancelled`, `always`, looking through nested calls). | 41 | `failure`, `cancelled`, `always`, looking through nested calls). | ||
| 42 | 42 | ||||
| 43 | + | ## Recorded (github.com, ermite-conformance corpus) | |||
| 44 | + | ||||
| 45 | + | Opt-in check against the corpus itself: `ERMITE_CORPUS=<ermite-conformance | |||
| 46 | + | checkout> go test ./workflow -run Corpus` (`corpus_test.go`) replays the | |||
| 47 | + | scenario files and compares the runs SkipCI, MatchPush and Parse predict with | |||
| 48 | + | the recordings. The cases copied into the unit tests stay. | |||
| 49 | + | ||||
| 50 | + | - Status functions (`success()`, `failure()`, `always()`, `cancelled()`) in | |||
| 51 | + | `env` or `run` expressions make the workflow invalid (recorded: | |||
| 52 | + | expr-status-in-env, expr-status-in-run, each a failed run with no job). The | |||
| 53 | + | text `Unrecognized function: 'success'. Located at position 1 within | |||
| 54 | + | expression: success()` is the one observed on the github.com run page, not in | |||
| 55 | + | the recordings; `TestRecordedInvalidWorkflows`. | |||
| 56 | + | - Invalid workflow messages (the recordings hold the failed run only, not the | |||
| 57 | + | text; the messages are those of the reference parser, which refuses the same | |||
| 58 | + | files): `The workflow must contain at least one job with | |||
| 59 | + | no dependencies.` (no root job: needs on an unknown job or a cycle with no | |||
| 60 | + | other root), `Job 'x' depends on unknown job 'y'.`, `Job 'x' depends on job | |||
| 61 | + | 'y' which creates a cycle in the dependency graph.`, `Required property is | |||
| 62 | + | missing: runs-on`, `Unexpected value 'unknown-key'` (invalid-needs, | |||
| 63 | + | invalid-schema). | |||
| 64 | + | ||||
| 43 | ## To verify (recordings) | 65 | ## To verify (recordings) | ||
| 44 | 66 | ||||
| 45 | - [ ] **Glob syntax** (`glob.go`): `*`, `**`, `?`, `+`, `[]`, `\`, `!`. The | 67 | - [ ] **Glob syntax** (`glob.go`): `*`, `**`, `?`, `+`, `[]`, `\`, `!`. The | ||
| 46 | cheat sheet's examples pass; `**/` matching nothing, `?` and `+` after a | 68 | cheat sheet's examples pass; `**/` matching nothing, `?` and `+` after a | ||
| 47 | `]`, `*` or an escaped character, `[!x]`, an unclosed `[` (literal here) | 69 | `]`, `*` or an escaped character, `[!x]`, an unclosed `[` (literal here) | ||
| 48 | and `/`-prefixed patterns are assumed. | 70 | and `/`-prefixed patterns are assumed. | ||
| 49 | - | - [ ] **`MatchPush`**: deletes never run; more than 3 tags run nothing for | 71 | + | - [x] **`MatchPush`** (recorded: push-tags-limit, filter-tags, |
| 50 | - | tags (not for the branch); branches-only filter and a tag push (no run) | 72 | + | filter-branches-ignore, filter-tag-branches-only, filter-paths-tag, |
| 51 | - | and the reverse; with both filters each ref uses its own; `paths` only | 73 | + | filter-paths*, push-delete-*): deletes never run; more than 3 tags run |
| 52 | - | for branches; first 300 files (also for `paths-ignore`); truncated list | 74 | + | nothing for tags (not for the branch); branches-only filter and a tag |
| 53 | - | passes; a push whose diff is empty never matches a `paths` filter and | 75 | + | push (no run) and the reverse; with both filters each ref uses its own; |
| 54 | - | always matches a `paths-ignore` one; unknown ref kinds never match. | 76 | + | `paths` only for branches; the first **3000** files, also for |
| 77 | + | `paths-ignore` (not 300). Still assumed: a push whose diff is empty | |||
| 78 | + | never matches a `paths` filter and always matches a `paths-ignore` one; | |||
| 79 | + | unknown ref kinds never match; truncated list passes. | |||
| 55 | - [ ] **`PushInfo.Created`** is not used: a new branch's file list is the | 80 | - [ ] **`PushInfo.Created`** is not used: a new branch's file list is the | ||
| 56 | caller's job (diff against the default branch merge-base). | 81 | caller's job (diff against the default branch merge-base). | ||
| 57 | - | - [ ] **`SkipCI`**: marker case, trailer rules (see its doc comment); whether | 82 | + | - [x] **`SkipCI`** (recorded: skip-directives, skip-directives-placement, |
| 58 | - | only the head commit counts; `[skip ci]` in the subject vs the body. | 83 | + | skip-checks-trailer, skip-directive-tag): markers are case-sensitive, |
| 59 | - | - [ ] **`ExpandMatrix`**: order of the product (first key slowest), include | 84 | + | anywhere; `skip-checks: true` needs two blank lines before it and |
| 85 | + | nothing after it, in exact case. The tag case is the caller's: the | |||
| 86 | + | message of the commit the tag points at. Assumed, not recorded: only | |||
| 87 | + | the head commit counts (the scenarios push one commit at a time); a | |||
| 88 | + | line of spaces or tabs is not a blank line; more than two blank lines | |||
| 89 | + | count like two. | |||
| 90 | + | - [ ] **`ExpandMatrix`**: the unknown exclude key error (once per offending | |||
| 91 | + | entry) and the over-256 message (`configurationLimitError`) are the | |||
| 92 | + | texts observed on the github.com run page for matrix-edge-values and | |||
| 93 | + | matrix-over-256, not in the recordings (a failed run with no job). To | |||
| 94 | + | verify: order of the product (first key slowest), include | |||
| 60 | semantics (added to every matching combination, never overwriting | 95 | semantics (added to every matching combination, never overwriting | ||
| 61 | original values, later includes overwrite earlier added values, a | 96 | original values, later includes overwrite earlier added values, a | ||
| 62 | non-matching include is a new combination, match by vector keys only), | 97 | non-matching include is a new combination, match by vector keys only), | ||
| @@ -64,18 +99,21 @@ code. | |||||
| 64 | combinations after include, key case-insensitivity, value comparison | 99 | combinations after include, key case-insensitivity, value comparison | ||
| 65 | (same kind; objects by containment), the error when all combinations are | 100 | (same kind; objects by containment), the error when all combinations are | ||
| 66 | excluded (GitHub may fail the run differently), empty vector error. | 101 | excluded (GitHub may fail the run differently), empty vector error. | ||
| 67 | - | - [ ] A **null matrix value** is shown as an empty segment (`job (, 1)`); | 102 | + | - [x] A **null or empty (`''`) matrix value** is not shown in the job name |
| 68 | - | GitHub may skip it. | 103 | + | (recorded: matrix-edge-values). |
| 69 | - [ ] **Job display names** (`MatrixCombo.DisplayName`, `Job.DisplayName`): | 104 | - [ ] **Job display names** (`MatrixCombo.DisplayName`, `Job.DisplayName`): | ||
| 70 | - | `name (v1, v2)` listing every scalar value of the combination in key | 105 | + | recorded in matrix-job-name and matrix-edge-values: `name (v1, v2)` |
| 71 | - | order, then keys added by include; a name that reads the `matrix` | 106 | + | listing the values in key order, numbers and booleans as |
| 107 | + | `CoerceString` shows them, a name that does not read `matrix` followed | |||
| 108 | + | by the values. Assumed: the keys added by include come after the vector | |||
| 109 | + | keys; arrays and objects are left out; a name that reads the `matrix` | |||
| 72 | context (found in the expression's syntax tree, not by searching the | 110 | context (found in the expression's syntax tree, not by searching the | ||
| 73 | - | text) is used as is. Booleans/numbers/null are shown with `CoerceString`. | 111 | + | text) is used as is. |
| 74 | - [ ] **`strategy` context** (`Strategy.Context`): `fail-fast`, `job-index`, | 112 | - [ ] **`strategy` context** (`Strategy.Context`): `fail-fast`, `job-index`, | ||
| 75 | `job-total`, `max-parallel` (the job count when unlimited). | 113 | `job-total`, `max-parallel` (the job count when unlimited). | ||
| 76 | - [ ] `fail-fast` and `max-parallel` accept strings (`"false"`, `"2"`) after | 114 | - [ ] `fail-fast` and `max-parallel` accept strings (`"false"`, `"2"`) after | ||
| 77 | evaluation; `max-parallel` must be a whole number ≥ 1. | 115 | evaluation; `max-parallel` must be a whole number ≥ 1. | ||
| 78 | - | - [ ] The 1 000-commit and 300-file limits themselves are the caller's. | 116 | + | - [ ] The 1 000-commit and 3 000-file limits themselves are the caller's. |
| 79 | 117 | ||||
| 80 | ## New in the second pass | 118 | ## New in the second pass | ||
| 81 | 119 | ||||
A workflow/corpus_test.go +237 -0
| @@ -0,0 +1,237 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package workflow | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "encoding/json" | |
| 7 | + | "fmt" | |
| 8 | + | "io/fs" | |
| 9 | + | "os" | |
| 10 | + | "path" | |
| 11 | + | "path/filepath" | |
| 12 | + | "slices" | |
| 13 | + | "strings" | |
| 14 | + | "testing" | |
| 15 | + | ||
| 16 | + | "go.yaml.in/yaml/v3" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | // The tests in this file check SkipCI, MatchPush and Parse against the | |
| 20 | + | // scenarios and the recordings of ermite-conformance. They are opt-in: set | |
| 21 | + | // ERMITE_CORPUS to the directory that holds corpus/ and recordings/ (a | |
| 22 | + | // checkout of ermite-conformance). The files are only read. | |
| 23 | + | // | |
| 24 | + | // ERMITE_CORPUS=/path/to/ermite-conformance go test ./workflow -run Corpus -v | |
| 25 | + | ||
| 26 | + | type corpusPush struct { | |
| 27 | + | Kind string `yaml:"kind"` | |
| 28 | + | Tree string `yaml:"tree"` | |
| 29 | + | Remove []string `yaml:"remove"` | |
| 30 | + | Message string `yaml:"message"` | |
| 31 | + | Count int `yaml:"count"` | |
| 32 | + | Generate []struct { | |
| 33 | + | Path string `yaml:"path"` | |
| 34 | + | Count int `yaml:"count"` | |
| 35 | + | } `yaml:"generate"` | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | type corpusScenario struct { | |
| 39 | + | Pushes []corpusPush `yaml:"pushes"` | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | type corpusRecording struct { | |
| 43 | + | Pushes []struct { | |
| 44 | + | Index int `json:"index"` | |
| 45 | + | Runs []struct { | |
| 46 | + | Workflow string `json:"workflow"` | |
| 47 | + | Conclusion string `json:"conclusion"` | |
| 48 | + | Jobs []json.RawMessage `json:"jobs"` | |
| 49 | + | } `json:"runs"` | |
| 50 | + | } `json:"pushes"` | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | // readTree returns the files under dir by path relative to dir. | |
| 54 | + | func readTree(t *testing.T, dir string) map[string]string { | |
| 55 | + | t.Helper() | |
| 56 | + | files := map[string]string{} | |
| 57 | + | err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { | |
| 58 | + | if err != nil || d.IsDir() { | |
| 59 | + | return err | |
| 60 | + | } | |
| 61 | + | b, err := os.ReadFile(p) | |
| 62 | + | if err != nil { | |
| 63 | + | return err | |
| 64 | + | } | |
| 65 | + | rel, _ := filepath.Rel(dir, p) | |
| 66 | + | files[filepath.ToSlash(rel)] = string(b) | |
| 67 | + | return nil | |
| 68 | + | }) | |
| 69 | + | if err != nil { | |
| 70 | + | t.Fatal(err) | |
| 71 | + | } | |
| 72 | + | return files | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func TestCorpusPushRules(t *testing.T) { | |
| 76 | + | root := os.Getenv("ERMITE_CORPUS") | |
| 77 | + | if root == "" { | |
| 78 | + | t.Skip("ERMITE_CORPUS is not set") | |
| 79 | + | } | |
| 80 | + | dirs, err := filepath.Glob(filepath.Join(root, "corpus", "*", "scenario.yml")) | |
| 81 | + | if err != nil || len(dirs) == 0 { | |
| 82 | + | t.Fatalf("no scenario in %s/corpus: %v", root, err) | |
| 83 | + | } | |
| 84 | + | checked := 0 | |
| 85 | + | for _, sc := range dirs { | |
| 86 | + | dir := filepath.Dir(sc) | |
| 87 | + | name := filepath.Base(dir) | |
| 88 | + | t.Run(name, func(t *testing.T) { | |
| 89 | + | var s corpusScenario | |
| 90 | + | b, err := os.ReadFile(sc) | |
| 91 | + | if err != nil { | |
| 92 | + | t.Fatal(err) | |
| 93 | + | } | |
| 94 | + | if err := yaml.Unmarshal(b, &s); err != nil { | |
| 95 | + | t.Fatal(err) | |
| 96 | + | } | |
| 97 | + | for _, p := range s.Pushes { | |
| 98 | + | if !slices.Contains([]string{"branch", "tag", "tags", "delete"}, p.Kind) { | |
| 99 | + | t.Skipf("push kind %q is not modelled", p.Kind) | |
| 100 | + | } | |
| 101 | + | } | |
| 102 | + | rb, err := os.ReadFile(filepath.Join(root, "recordings", name+".json")) | |
| 103 | + | if err != nil { | |
| 104 | + | t.Skipf("no recording: %v", err) | |
| 105 | + | } | |
| 106 | + | var rec corpusRecording | |
| 107 | + | if err := json.Unmarshal(rb, &rec); err != nil { | |
| 108 | + | t.Fatal(err) | |
| 109 | + | } | |
| 110 | + | checked++ | |
| 111 | + | checkScenario(t, dir, name, s, rec) | |
| 112 | + | }) | |
| 113 | + | } | |
| 114 | + | if checked == 0 { | |
| 115 | + | t.Error("no scenario was checked") | |
| 116 | + | } | |
| 117 | + | } | |
| 118 | + | ||
| 119 | + | func checkScenario(t *testing.T, dir, name string, s corpusScenario, rec corpusRecording) { | |
| 120 | + | state := map[string]string{} // the files of the head commit | |
| 121 | + | for i, p := range s.Pushes { | |
| 122 | + | n := i + 1 | |
| 123 | + | var changed []string | |
| 124 | + | if p.Kind != "delete" { | |
| 125 | + | tree := p.Tree | |
| 126 | + | if tree == "" && i == 0 { | |
| 127 | + | tree = "tree" | |
| 128 | + | } | |
| 129 | + | next := cloneFiles(state) | |
| 130 | + | if tree != "" { | |
| 131 | + | for f, c := range readTree(t, filepath.Join(dir, tree)) { | |
| 132 | + | next[f] = c | |
| 133 | + | } | |
| 134 | + | } | |
| 135 | + | for _, f := range p.Remove { | |
| 136 | + | delete(next, f) | |
| 137 | + | } | |
| 138 | + | for _, g := range p.Generate { | |
| 139 | + | for k := 1; k <= g.Count; k++ { | |
| 140 | + | next[strings.ReplaceAll(g.Path, "{n}", fmt.Sprint(k))] = "generated" | |
| 141 | + | } | |
| 142 | + | } | |
| 143 | + | for f, c := range next { | |
| 144 | + | if old, ok := state[f]; !ok || old != c { | |
| 145 | + | changed = append(changed, f) | |
| 146 | + | } | |
| 147 | + | } | |
| 148 | + | for f := range state { | |
| 149 | + | if _, ok := next[f]; !ok { | |
| 150 | + | changed = append(changed, f) | |
| 151 | + | } | |
| 152 | + | } | |
| 153 | + | slices.Sort(changed) | |
| 154 | + | state = next | |
| 155 | + | } | |
| 156 | + | message := p.Message | |
| 157 | + | if message == "" { | |
| 158 | + | message = fmt.Sprintf("scenario %s push %d", name, n) | |
| 159 | + | } | |
| 160 | + | ||
| 161 | + | info := PushInfo{Created: i == 0 && p.Kind == "branch", ChangedFiles: changed} | |
| 162 | + | refs := 1 | |
| 163 | + | switch p.Kind { | |
| 164 | + | case "branch": | |
| 165 | + | info.Ref = "refs/heads/scn/" + name + "/r1" | |
| 166 | + | case "tag", "tags": | |
| 167 | + | refs = max(p.Count, 1) | |
| 168 | + | info.Ref = "refs/tags/scn-" + name + "-r1-1" | |
| 169 | + | info.TagCount = refs | |
| 170 | + | info.Created = true | |
| 171 | + | case "delete": | |
| 172 | + | // Deletes the last ref pushed. | |
| 173 | + | prev := s.Pushes[i-1] | |
| 174 | + | info.Deleted = true | |
| 175 | + | if prev.Kind == "branch" { | |
| 176 | + | info.Ref = "refs/heads/scn/" + name + "/r1" | |
| 177 | + | } else { | |
| 178 | + | info.Ref = "refs/tags/scn-" + name + "-r1-1" | |
| 179 | + | info.TagCount = 1 | |
| 180 | + | } | |
| 181 | + | } | |
| 182 | + | ||
| 183 | + | got := map[string]int{} // runs by workflow file in the recording | |
| 184 | + | var failed []string // workflows recorded as a failed run without job | |
| 185 | + | for _, rp := range rec.Pushes { | |
| 186 | + | if rp.Index != n { | |
| 187 | + | continue | |
| 188 | + | } | |
| 189 | + | for _, r := range rp.Runs { | |
| 190 | + | got[r.Workflow]++ | |
| 191 | + | if r.Conclusion == "failure" && len(r.Jobs) == 0 { | |
| 192 | + | failed = append(failed, r.Workflow) | |
| 193 | + | } | |
| 194 | + | } | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | skip := p.Kind != "delete" && SkipCI(message) | |
| 198 | + | var files []string | |
| 199 | + | for f := range state { | |
| 200 | + | if strings.HasPrefix(f, ".github/workflows/") && path.Dir(f) == ".github/workflows" { | |
| 201 | + | files = append(files, f) | |
| 202 | + | } | |
| 203 | + | } | |
| 204 | + | slices.Sort(files) | |
| 205 | + | for _, f := range files { | |
| 206 | + | w, errs := Parse(f, []byte(state[f])) | |
| 207 | + | want := 0 | |
| 208 | + | switch { | |
| 209 | + | case p.Kind == "delete" || skip: | |
| 210 | + | case len(errs) > 0: | |
| 211 | + | // An invalid file gives one failed run without job. | |
| 212 | + | want = 1 | |
| 213 | + | if !slices.Contains(failed, f) { | |
| 214 | + | t.Errorf("push %d (%q): %s is invalid (%v) but the recording has no failed run for it", n, message, f, errs[0]) | |
| 215 | + | } | |
| 216 | + | case w.MatchPush(info): | |
| 217 | + | want = refs | |
| 218 | + | } | |
| 219 | + | if got[f] != want { | |
| 220 | + | t.Errorf("push %d (%q, %s): %s: GitHub started %d run(s), we expect %d", n, message, p.Kind, f, got[f], want) | |
| 221 | + | } | |
| 222 | + | } | |
| 223 | + | for f, c := range got { | |
| 224 | + | if !slices.Contains(files, f) { | |
| 225 | + | t.Errorf("push %d: recording has %d run(s) of %s, which is not in the scenario", n, c, f) | |
| 226 | + | } | |
| 227 | + | } | |
| 228 | + | } | |
| 229 | + | } | |
| 230 | + | ||
| 231 | + | func cloneFiles(m map[string]string) map[string]string { | |
| 232 | + | out := make(map[string]string, len(m)) | |
| 233 | + | for k, v := range m { | |
| 234 | + | out[k] = v | |
| 235 | + | } | |
| 236 | + | return out | |
| 237 | + | } | |
| @@ -0,0 +1,237 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package workflow | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "encoding/json" | |||
| 7 | + | "fmt" | |||
| 8 | + | "io/fs" | |||
| 9 | + | "os" | |||
| 10 | + | "path" | |||
| 11 | + | "path/filepath" | |||
| 12 | + | "slices" | |||
| 13 | + | "strings" | |||
| 14 | + | "testing" | |||
| 15 | + | ||||
| 16 | + | "go.yaml.in/yaml/v3" | |||
| 17 | + | ) | |||
| 18 | + | ||||
| 19 | + | // The tests in this file check SkipCI, MatchPush and Parse against the | |||
| 20 | + | // scenarios and the recordings of ermite-conformance. They are opt-in: set | |||
| 21 | + | // ERMITE_CORPUS to the directory that holds corpus/ and recordings/ (a | |||
| 22 | + | // checkout of ermite-conformance). The files are only read. | |||
| 23 | + | // | |||
| 24 | + | // ERMITE_CORPUS=/path/to/ermite-conformance go test ./workflow -run Corpus -v | |||
| 25 | + | ||||
| 26 | + | type corpusPush struct { | |||
| 27 | + | Kind string `yaml:"kind"` | |||
| 28 | + | Tree string `yaml:"tree"` | |||
| 29 | + | Remove []string `yaml:"remove"` | |||
| 30 | + | Message string `yaml:"message"` | |||
| 31 | + | Count int `yaml:"count"` | |||
| 32 | + | Generate []struct { | |||
| 33 | + | Path string `yaml:"path"` | |||
| 34 | + | Count int `yaml:"count"` | |||
| 35 | + | } `yaml:"generate"` | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | type corpusScenario struct { | |||
| 39 | + | Pushes []corpusPush `yaml:"pushes"` | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | type corpusRecording struct { | |||
| 43 | + | Pushes []struct { | |||
| 44 | + | Index int `json:"index"` | |||
| 45 | + | Runs []struct { | |||
| 46 | + | Workflow string `json:"workflow"` | |||
| 47 | + | Conclusion string `json:"conclusion"` | |||
| 48 | + | Jobs []json.RawMessage `json:"jobs"` | |||
| 49 | + | } `json:"runs"` | |||
| 50 | + | } `json:"pushes"` | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | // readTree returns the files under dir by path relative to dir. | |||
| 54 | + | func readTree(t *testing.T, dir string) map[string]string { | |||
| 55 | + | t.Helper() | |||
| 56 | + | files := map[string]string{} | |||
| 57 | + | err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { | |||
| 58 | + | if err != nil || d.IsDir() { | |||
| 59 | + | return err | |||
| 60 | + | } | |||
| 61 | + | b, err := os.ReadFile(p) | |||
| 62 | + | if err != nil { | |||
| 63 | + | return err | |||
| 64 | + | } | |||
| 65 | + | rel, _ := filepath.Rel(dir, p) | |||
| 66 | + | files[filepath.ToSlash(rel)] = string(b) | |||
| 67 | + | return nil | |||
| 68 | + | }) | |||
| 69 | + | if err != nil { | |||
| 70 | + | t.Fatal(err) | |||
| 71 | + | } | |||
| 72 | + | return files | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func TestCorpusPushRules(t *testing.T) { | |||
| 76 | + | root := os.Getenv("ERMITE_CORPUS") | |||
| 77 | + | if root == "" { | |||
| 78 | + | t.Skip("ERMITE_CORPUS is not set") | |||
| 79 | + | } | |||
| 80 | + | dirs, err := filepath.Glob(filepath.Join(root, "corpus", "*", "scenario.yml")) | |||
| 81 | + | if err != nil || len(dirs) == 0 { | |||
| 82 | + | t.Fatalf("no scenario in %s/corpus: %v", root, err) | |||
| 83 | + | } | |||
| 84 | + | checked := 0 | |||
| 85 | + | for _, sc := range dirs { | |||
| 86 | + | dir := filepath.Dir(sc) | |||
| 87 | + | name := filepath.Base(dir) | |||
| 88 | + | t.Run(name, func(t *testing.T) { | |||
| 89 | + | var s corpusScenario | |||
| 90 | + | b, err := os.ReadFile(sc) | |||
| 91 | + | if err != nil { | |||
| 92 | + | t.Fatal(err) | |||
| 93 | + | } | |||
| 94 | + | if err := yaml.Unmarshal(b, &s); err != nil { | |||
| 95 | + | t.Fatal(err) | |||
| 96 | + | } | |||
| 97 | + | for _, p := range s.Pushes { | |||
| 98 | + | if !slices.Contains([]string{"branch", "tag", "tags", "delete"}, p.Kind) { | |||
| 99 | + | t.Skipf("push kind %q is not modelled", p.Kind) | |||
| 100 | + | } | |||
| 101 | + | } | |||
| 102 | + | rb, err := os.ReadFile(filepath.Join(root, "recordings", name+".json")) | |||
| 103 | + | if err != nil { | |||
| 104 | + | t.Skipf("no recording: %v", err) | |||
| 105 | + | } | |||
| 106 | + | var rec corpusRecording | |||
| 107 | + | if err := json.Unmarshal(rb, &rec); err != nil { | |||
| 108 | + | t.Fatal(err) | |||
| 109 | + | } | |||
| 110 | + | checked++ | |||
| 111 | + | checkScenario(t, dir, name, s, rec) | |||
| 112 | + | }) | |||
| 113 | + | } | |||
| 114 | + | if checked == 0 { | |||
| 115 | + | t.Error("no scenario was checked") | |||
| 116 | + | } | |||
| 117 | + | } | |||
| 118 | + | ||||
| 119 | + | func checkScenario(t *testing.T, dir, name string, s corpusScenario, rec corpusRecording) { | |||
| 120 | + | state := map[string]string{} // the files of the head commit | |||
| 121 | + | for i, p := range s.Pushes { | |||
| 122 | + | n := i + 1 | |||
| 123 | + | var changed []string | |||
| 124 | + | if p.Kind != "delete" { | |||
| 125 | + | tree := p.Tree | |||
| 126 | + | if tree == "" && i == 0 { | |||
| 127 | + | tree = "tree" | |||
| 128 | + | } | |||
| 129 | + | next := cloneFiles(state) | |||
| 130 | + | if tree != "" { | |||
| 131 | + | for f, c := range readTree(t, filepath.Join(dir, tree)) { | |||
| 132 | + | next[f] = c | |||
| 133 | + | } | |||
| 134 | + | } | |||
| 135 | + | for _, f := range p.Remove { | |||
| 136 | + | delete(next, f) | |||
| 137 | + | } | |||
| 138 | + | for _, g := range p.Generate { | |||
| 139 | + | for k := 1; k <= g.Count; k++ { | |||
| 140 | + | next[strings.ReplaceAll(g.Path, "{n}", fmt.Sprint(k))] = "generated" | |||
| 141 | + | } | |||
| 142 | + | } | |||
| 143 | + | for f, c := range next { | |||
| 144 | + | if old, ok := state[f]; !ok || old != c { | |||
| 145 | + | changed = append(changed, f) | |||
| 146 | + | } | |||
| 147 | + | } | |||
| 148 | + | for f := range state { | |||
| 149 | + | if _, ok := next[f]; !ok { | |||
| 150 | + | changed = append(changed, f) | |||
| 151 | + | } | |||
| 152 | + | } | |||
| 153 | + | slices.Sort(changed) | |||
| 154 | + | state = next | |||
| 155 | + | } | |||
| 156 | + | message := p.Message | |||
| 157 | + | if message == "" { | |||
| 158 | + | message = fmt.Sprintf("scenario %s push %d", name, n) | |||
| 159 | + | } | |||
| 160 | + | ||||
| 161 | + | info := PushInfo{Created: i == 0 && p.Kind == "branch", ChangedFiles: changed} | |||
| 162 | + | refs := 1 | |||
| 163 | + | switch p.Kind { | |||
| 164 | + | case "branch": | |||
| 165 | + | info.Ref = "refs/heads/scn/" + name + "/r1" | |||
| 166 | + | case "tag", "tags": | |||
| 167 | + | refs = max(p.Count, 1) | |||
| 168 | + | info.Ref = "refs/tags/scn-" + name + "-r1-1" | |||
| 169 | + | info.TagCount = refs | |||
| 170 | + | info.Created = true | |||
| 171 | + | case "delete": | |||
| 172 | + | // Deletes the last ref pushed. | |||
| 173 | + | prev := s.Pushes[i-1] | |||
| 174 | + | info.Deleted = true | |||
| 175 | + | if prev.Kind == "branch" { | |||
| 176 | + | info.Ref = "refs/heads/scn/" + name + "/r1" | |||
| 177 | + | } else { | |||
| 178 | + | info.Ref = "refs/tags/scn-" + name + "-r1-1" | |||
| 179 | + | info.TagCount = 1 | |||
| 180 | + | } | |||
| 181 | + | } | |||
| 182 | + | ||||
| 183 | + | got := map[string]int{} // runs by workflow file in the recording | |||
| 184 | + | var failed []string // workflows recorded as a failed run without job | |||
| 185 | + | for _, rp := range rec.Pushes { | |||
| 186 | + | if rp.Index != n { | |||
| 187 | + | continue | |||
| 188 | + | } | |||
| 189 | + | for _, r := range rp.Runs { | |||
| 190 | + | got[r.Workflow]++ | |||
| 191 | + | if r.Conclusion == "failure" && len(r.Jobs) == 0 { | |||
| 192 | + | failed = append(failed, r.Workflow) | |||
| 193 | + | } | |||
| 194 | + | } | |||
| 195 | + | } | |||
| 196 | + | ||||
| 197 | + | skip := p.Kind != "delete" && SkipCI(message) | |||
| 198 | + | var files []string | |||
| 199 | + | for f := range state { | |||
| 200 | + | if strings.HasPrefix(f, ".github/workflows/") && path.Dir(f) == ".github/workflows" { | |||
| 201 | + | files = append(files, f) | |||
| 202 | + | } | |||
| 203 | + | } | |||
| 204 | + | slices.Sort(files) | |||
| 205 | + | for _, f := range files { | |||
| 206 | + | w, errs := Parse(f, []byte(state[f])) | |||
| 207 | + | want := 0 | |||
| 208 | + | switch { | |||
| 209 | + | case p.Kind == "delete" || skip: | |||
| 210 | + | case len(errs) > 0: | |||
| 211 | + | // An invalid file gives one failed run without job. | |||
| 212 | + | want = 1 | |||
| 213 | + | if !slices.Contains(failed, f) { | |||
| 214 | + | t.Errorf("push %d (%q): %s is invalid (%v) but the recording has no failed run for it", n, message, f, errs[0]) | |||
| 215 | + | } | |||
| 216 | + | case w.MatchPush(info): | |||
| 217 | + | want = refs | |||
| 218 | + | } | |||
| 219 | + | if got[f] != want { | |||
| 220 | + | t.Errorf("push %d (%q, %s): %s: GitHub started %d run(s), we expect %d", n, message, p.Kind, f, got[f], want) | |||
| 221 | + | } | |||
| 222 | + | } | |||
| 223 | + | for f, c := range got { | |||
| 224 | + | if !slices.Contains(files, f) { | |||
| 225 | + | t.Errorf("push %d: recording has %d run(s) of %s, which is not in the scenario", n, c, f) | |||
| 226 | + | } | |||
| 227 | + | } | |||
| 228 | + | } | |||
| 229 | + | } | |||
| 230 | + | ||||
| 231 | + | func cloneFiles(m map[string]string) map[string]string { | |||
| 232 | + | out := make(map[string]string, len(m)) | |||
| 233 | + | for k, v := range m { | |||
| 234 | + | out[k] = v | |||
| 235 | + | } | |||
| 236 | + | return out | |||
| 237 | + | } | |||
M workflow/glob.go +5 -4
| @@ -24,10 +24,11 @@ import ( | |||
| 24 | 24 | // Every other character, "." included, stands for itself. Matching is | |
| 25 | 25 | // case-sensitive and covers the whole name. | |
| 26 | 26 | // | |
| 27 | - | // TODO(verify with recording): the exact GitHub implementation is not | |
| 28 | - | // public. The cheat sheet fixes the common cases; "**/" matching nothing, | |
| 29 | - | // "\" escapes and how ? and + behave after a wildcard or a bracket are | |
| 30 | - | // reconstructed. | |
| 27 | + | // Recorded on github.com (filter-branches, filter-paths, filter-paths-ignore, | |
| 28 | + | // see TestMatchPushRecordedGlobs): "*", "**", "scn/filter-*/**", "**.md", | |
| 29 | + | // "*.md" and negation order. TODO(verify with recording): "**/" matching | |
| 30 | + | // nothing, "\" escapes and how ? and + behave after a wildcard or a bracket | |
| 31 | + | // are reconstructed. | |
| 31 | 32 | type Pattern struct { | |
| 32 | 33 | // Negated is set for a pattern that started with "!". | |
| 33 | 34 | Negated bool | |
| @@ -24,10 +24,11 @@ import ( | |||||
| 24 | // Every other character, "." included, stands for itself. Matching is | 24 | // Every other character, "." included, stands for itself. Matching is | ||
| 25 | // case-sensitive and covers the whole name. | 25 | // case-sensitive and covers the whole name. | ||
| 26 | // | 26 | // | ||
| 27 | - | // TODO(verify with recording): the exact GitHub implementation is not | 27 | + | // Recorded on github.com (filter-branches, filter-paths, filter-paths-ignore, |
| 28 | - | // public. The cheat sheet fixes the common cases; "**/" matching nothing, | 28 | + | // see TestMatchPushRecordedGlobs): "*", "**", "scn/filter-*/**", "**.md", |
| 29 | - | // "\" escapes and how ? and + behave after a wildcard or a bracket are | 29 | + | // "*.md" and negation order. TODO(verify with recording): "**/" matching |
| 30 | - | // reconstructed. | 30 | + | // nothing, "\" escapes and how ? and + behave after a wildcard or a bracket |
| 31 | + | // are reconstructed. | |||
| 31 | type Pattern struct { | 32 | type Pattern struct { | ||
| 32 | // Negated is set for a pattern that started with "!". | 33 | // Negated is set for a pattern that started with "!". | ||
| 33 | Negated bool | 34 | Negated bool | ||
M workflow/matrix.go +86 -32
| @@ -34,8 +34,9 @@ type MatrixCombo struct { | |||
| 34 | 34 | // they were first added. | |
| 35 | 35 | Values *expr.Dictionary | |
| 36 | 36 | // Segments are the scalar values of Values, in order, shown in the name | |
| 37 | - | // of the job: "build (ubuntu-latest, 14)". Arrays and mappings are left | |
| 38 | - | // out; null shows as an empty segment. | |
| 37 | + | // of the job: "build (ubuntu-latest, 14)". Null and the empty string are | |
| 38 | + | // left out, as recorded (matrix-edge-values); arrays and mappings are | |
| 39 | + | // left out too, which is assumed. | |
| 39 | 40 | Segments []string | |
| 40 | 41 | } | |
| 41 | 42 | ||
| @@ -69,16 +70,19 @@ func (c MatrixCombo) DisplayName(base string) string { | |||
| 69 | 70 | // [MaxMatrixProduct], include and exclude may have [MaxMatrixEntries] entries | |
| 70 | 71 | // each, and ctx is checked every few thousand combinations. | |
| 71 | 72 | // | |
| 72 | - | // TODO(verify with recording): the documented semantics of include and | |
| 73 | - | // exclude are implemented as documented, but the following are assumed: the | |
| 74 | - | // order of the cross product (first key slowest), the display name listing | |
| 75 | - | // every scalar value including those added by include (see | |
| 76 | - | // [MatrixCombo.DisplayName]) and showing null as an empty segment, the | |
| 77 | - | // comparison of values (same kind, strings compared exactly, mappings by | |
| 78 | - | // containment), an include key matching a vector key case-insensitively, the | |
| 79 | - | // error when every combination is excluded, the limits above, and the | |
| 80 | - | // wording of all errors except the unknown exclude key, which is GitHub's | |
| 81 | - | // (also to verify). | |
| 73 | + | // What github.com settled. Recorded (matrix-edge-values): null and the empty | |
| 74 | + | // string are left out of the default job name. For matrix-edge-values (exclude key not | |
| 75 | + | // in the matrix) and matrix-over-256 the recordings hold only a failed run | |
| 76 | + | // with no job: the wording of the two errors below, and the unknown exclude | |
| 77 | + | // key being reported once per offending entry, were observed on the github.com | |
| 78 | + | // run page, not in the recordings. Still assumed, from the documented | |
| 79 | + | // semantics of include and exclude: the order of the cross product (first key | |
| 80 | + | // slowest), the display name listing every scalar value including those | |
| 81 | + | // added by include (see [MatrixCombo.DisplayName]), the comparison of values | |
| 82 | + | // (same kind, strings compared exactly, mappings by containment), an include | |
| 83 | + | // key matching a vector key case-insensitively, the error when every | |
| 84 | + | // combination is excluded, the other limits above, and the wording of the | |
| 85 | + | // other errors. | |
| 82 | 86 | func ExpandMatrix(ctx context.Context, strategy expr.Value) ([]MatrixCombo, error) { | |
| 83 | 87 | s, err := ParseStrategy(ctx, strategy) | |
| 84 | 88 | if err != nil { | |
| @@ -121,7 +125,7 @@ func buildMatrix(ctx context.Context, m expr.Value) ([]MatrixCombo, bool, error) | |||
| 121 | 125 | return nil, false, fmt.Errorf("matrix vector '%s' does not contain any values", p.Key) | |
| 122 | 126 | } | |
| 123 | 127 | if arr.Len() > MaxMatrixProduct { | |
| 124 | - | return nil, false, errTooBig | |
| 128 | + | return nil, false, errProductTooBig | |
| 125 | 129 | } | |
| 126 | 130 | vecs = append(vecs, vector{key: p.Key, items: arr.Items()}) | |
| 127 | 131 | } | |
| @@ -153,7 +157,32 @@ type message string | |||
| 153 | 157 | ||
| 154 | 158 | func (m message) Error() string { return string(m) } | |
| 155 | 159 | ||
| 156 | - | var errTooBig = fmt.Errorf("matrix size limit of %d exceeded", MaxMatrixJobs) | |
| 160 | + | // errProductTooBig is returned when the cross product of the vectors is too | |
| 161 | + | // large to be enumerated. GitHub would count the configurations left after | |
| 162 | + | // the exclusions; this limit keeps the work bounded, see [MaxMatrixProduct]. | |
| 163 | + | var errProductTooBig = fmt.Errorf("matrix too large: the cross product of the vectors exceeds %d combinations", MaxMatrixProduct) | |
| 164 | + | ||
| 165 | + | // configurationLimitError is returned when a matrix produces more than | |
| 166 | + | // [MaxMatrixJobs] configurations. Its text is the one observed on the | |
| 167 | + | // github.com run page for matrix-over-256 (not in the recordings, which only | |
| 168 | + | // hold the failed run): "Strategy for job 'too-many' produced 272 | |
| 169 | + | // configurations which exceeds the maximum of 256 configurations". Job is | |
| 170 | + | // empty when the matrix was expanded without a job, see | |
| 171 | + | // [Job.EvaluateStrategy]. | |
| 172 | + | type configurationLimitError struct { | |
| 173 | + | // Job is the id of the job. | |
| 174 | + | Job string | |
| 175 | + | // Count is the number of configurations the matrix produces. | |
| 176 | + | Count int | |
| 177 | + | } | |
| 178 | + | ||
| 179 | + | func (e *configurationLimitError) Error() string { | |
| 180 | + | subject := "Strategy" | |
| 181 | + | if e.Job != "" { | |
| 182 | + | subject = fmt.Sprintf("Strategy for job '%s'", e.Job) | |
| 183 | + | } | |
| 184 | + | return fmt.Sprintf("%s produced %d configurations which exceeds the maximum of %d configurations", subject, e.Count, MaxMatrixJobs) | |
| 185 | + | } | |
| 157 | 186 | ||
| 158 | 187 | type vector struct { | |
| 159 | 188 | key string | |
| @@ -188,19 +217,27 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 188 | 217 | // Exclude entries become lists of conditions on item positions, so that | |
| 189 | 218 | // testing a combination costs a few array reads. | |
| 190 | 219 | var excl [][]condition | |
| 220 | + | var bad []error | |
| 191 | 221 | for _, ex := range excludes { | |
| 192 | 222 | var conds []condition | |
| 223 | + | unknown := false | |
| 193 | 224 | for _, p := range ex.Pairs() { | |
| 194 | 225 | vi, ok := index[strings.ToLower(p.Key)] | |
| 195 | 226 | if !ok { | |
| 196 | - | return nil, false, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key)) | |
| 227 | + | // One error per offending entry, for its first unknown key. | |
| 228 | + | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | |
| 229 | + | unknown = true | |
| 230 | + | break | |
| 197 | 231 | } | |
| 198 | 232 | conds = append(conds, compileCondition(vecs, vi, p.Value)) | |
| 199 | 233 | } | |
| 200 | - | if len(conds) > 0 { | |
| 234 | + | if len(conds) > 0 && !unknown { | |
| 201 | 235 | excl = append(excl, conds) | |
| 202 | 236 | } | |
| 203 | 237 | } | |
| 238 | + | if len(bad) > 0 { | |
| 239 | + | return nil, false, errors.Join(bad...) | |
| 240 | + | } | |
| 204 | 241 | incl := make([]includeEntry, len(includes)) | |
| 205 | 242 | for n, inc := range includes { | |
| 206 | 243 | for _, p := range inc.Pairs() { | |
| @@ -212,14 +249,17 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 212 | 249 | } | |
| 213 | 250 | } | |
| 214 | 251 | ||
| 252 | + | // total counts every configuration, so that the error of a matrix over the | |
| 253 | + | // limit tells how many it produces; only the first MaxMatrixJobs are built. | |
| 215 | 254 | var combos []MatrixCombo | |
| 255 | + | total := 0 | |
| 216 | 256 | matched := make([]bool, len(includes)) | |
| 217 | 257 | if len(vecs) > 0 { | |
| 218 | 258 | product := 1 | |
| 219 | 259 | for _, v := range vecs { | |
| 220 | 260 | product *= len(v.items) | |
| 221 | 261 | if product > MaxMatrixProduct { | |
| 222 | - | return nil, false, errTooBig | |
| 262 | + | return nil, false, errProductTooBig | |
| 223 | 263 | } | |
| 224 | 264 | } | |
| 225 | 265 | tuple := make([]int, len(vecs)) | |
| @@ -245,22 +285,28 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 245 | 285 | continue next | |
| 246 | 286 | } | |
| 247 | 287 | } | |
| 248 | - | b := &comboBuilder{vals: map[string]expr.Value{}} | |
| 249 | - | for k, v := range vecs { | |
| 250 | - | b.set(v.key, v.items[tuple[k]]) | |
| 288 | + | total++ | |
| 289 | + | build := total <= MaxMatrixJobs | |
| 290 | + | var b *comboBuilder | |
| 291 | + | if build { | |
| 292 | + | b = &comboBuilder{vals: map[string]expr.Value{}} | |
| 293 | + | for k, v := range vecs { | |
| 294 | + | b.set(v.key, v.items[tuple[k]]) | |
| 295 | + | } | |
| 251 | 296 | } | |
| 252 | 297 | for n, inc := range incl { | |
| 253 | 298 | if !allHold(inc.conds, tuple) { | |
| 254 | 299 | continue | |
| 255 | 300 | } | |
| 256 | 301 | matched[n] = true | |
| 257 | - | for _, p := range inc.extra { | |
| 258 | - | b.set(p.Key, p.Value) | |
| 302 | + | if build { | |
| 303 | + | for _, p := range inc.extra { | |
| 304 | + | b.set(p.Key, p.Value) | |
| 305 | + | } | |
| 259 | 306 | } | |
| 260 | 307 | } | |
| 261 | - | combos = append(combos, b.combo()) | |
| 262 | - | if len(combos) > MaxMatrixJobs { | |
| 263 | - | return nil, false, errTooBig | |
| 308 | + | if build { | |
| 309 | + | combos = append(combos, b.combo()) | |
| 264 | 310 | } | |
| 265 | 311 | } | |
| 266 | 312 | } | |
| @@ -268,14 +314,18 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 268 | 314 | if matched[n] { | |
| 269 | 315 | continue | |
| 270 | 316 | } | |
| 317 | + | total++ | |
| 318 | + | if total > MaxMatrixJobs { | |
| 319 | + | continue | |
| 320 | + | } | |
| 271 | 321 | b := &comboBuilder{vals: map[string]expr.Value{}} | |
| 272 | 322 | for _, p := range inc.Pairs() { | |
| 273 | 323 | b.set(p.Key, p.Value) | |
| 274 | 324 | } | |
| 275 | 325 | combos = append(combos, b.combo()) | |
| 276 | - | if len(combos) > MaxMatrixJobs { | |
| 277 | - | return nil, false, errTooBig | |
| 278 | - | } | |
| 326 | + | } | |
| 327 | + | if total > MaxMatrixJobs { | |
| 328 | + | return nil, false, &configurationLimitError{Count: total} | |
| 279 | 329 | } | |
| 280 | 330 | if len(combos) == 0 { | |
| 281 | 331 | if len(vecs) == 0 && len(includes) == 0 && len(excludes) == 0 { | |
| @@ -368,12 +418,16 @@ func valuesMatch(have, want expr.Value) bool { | |||
| 368 | 418 | return false | |
| 369 | 419 | } | |
| 370 | 420 | ||
| 371 | - | // segment is the text a matrix value adds to the name of a job; arrays and | |
| 372 | - | // mappings add nothing. | |
| 421 | + | // segment is the text a matrix value adds to the name of a job. Null and the | |
| 422 | + | // empty string add nothing, numbers and booleans are shown as in an | |
| 423 | + | // expression ("1.5", "true"); all recorded (matrix-edge-values: | |
| 424 | + | // "default-name", "two-axes (x)"). Arrays and mappings add nothing: assumed, | |
| 425 | + | // not recorded. | |
| 373 | 426 | func segment(v expr.Value) (string, bool) { | |
| 374 | 427 | switch v.(type) { | |
| 375 | - | case *expr.Array, *expr.Dictionary: | |
| 428 | + | case nil, expr.Null, *expr.Array, *expr.Dictionary: | |
| 376 | 429 | return "", false | |
| 377 | 430 | } | |
| 378 | - | return expr.CoerceString(v), true | |
| 431 | + | s := expr.CoerceString(v) | |
| 432 | + | return s, s != "" | |
| 379 | 433 | } | |
| @@ -34,8 +34,9 @@ type MatrixCombo struct { | |||||
| 34 | // they were first added. | 34 | // they were first added. | ||
| 35 | Values *expr.Dictionary | 35 | Values *expr.Dictionary | ||
| 36 | // Segments are the scalar values of Values, in order, shown in the name | 36 | // Segments are the scalar values of Values, in order, shown in the name | ||
| 37 | - | // of the job: "build (ubuntu-latest, 14)". Arrays and mappings are left | 37 | + | // of the job: "build (ubuntu-latest, 14)". Null and the empty string are |
| 38 | - | // out; null shows as an empty segment. | 38 | + | // left out, as recorded (matrix-edge-values); arrays and mappings are |
| 39 | + | // left out too, which is assumed. | |||
| 39 | Segments []string | 40 | Segments []string | ||
| 40 | } | 41 | } | ||
| 41 | 42 | ||||
| @@ -69,16 +70,19 @@ func (c MatrixCombo) DisplayName(base string) string { | |||||
| 69 | // [MaxMatrixProduct], include and exclude may have [MaxMatrixEntries] entries | 70 | // [MaxMatrixProduct], include and exclude may have [MaxMatrixEntries] entries | ||
| 70 | // each, and ctx is checked every few thousand combinations. | 71 | // each, and ctx is checked every few thousand combinations. | ||
| 71 | // | 72 | // | ||
| 72 | - | // TODO(verify with recording): the documented semantics of include and | 73 | + | // What github.com settled. Recorded (matrix-edge-values): null and the empty |
| 73 | - | // exclude are implemented as documented, but the following are assumed: the | 74 | + | // string are left out of the default job name. For matrix-edge-values (exclude key not |
| 74 | - | // order of the cross product (first key slowest), the display name listing | 75 | + | // in the matrix) and matrix-over-256 the recordings hold only a failed run |
| 75 | - | // every scalar value including those added by include (see | 76 | + | // with no job: the wording of the two errors below, and the unknown exclude |
| 76 | - | // [MatrixCombo.DisplayName]) and showing null as an empty segment, the | 77 | + | // key being reported once per offending entry, were observed on the github.com |
| 77 | - | // comparison of values (same kind, strings compared exactly, mappings by | 78 | + | // run page, not in the recordings. Still assumed, from the documented |
| 78 | - | // containment), an include key matching a vector key case-insensitively, the | 79 | + | // semantics of include and exclude: the order of the cross product (first key |
| 79 | - | // error when every combination is excluded, the limits above, and the | 80 | + | // slowest), the display name listing every scalar value including those |
| 80 | - | // wording of all errors except the unknown exclude key, which is GitHub's | 81 | + | // added by include (see [MatrixCombo.DisplayName]), the comparison of values |
| 81 | - | // (also to verify). | 82 | + | // (same kind, strings compared exactly, mappings by containment), an include |
| 83 | + | // key matching a vector key case-insensitively, the error when every | |||
| 84 | + | // combination is excluded, the other limits above, and the wording of the | |||
| 85 | + | // other errors. | |||
| 82 | func ExpandMatrix(ctx context.Context, strategy expr.Value) ([]MatrixCombo, error) { | 86 | func ExpandMatrix(ctx context.Context, strategy expr.Value) ([]MatrixCombo, error) { | ||
| 83 | s, err := ParseStrategy(ctx, strategy) | 87 | s, err := ParseStrategy(ctx, strategy) | ||
| 84 | if err != nil { | 88 | if err != nil { | ||
| @@ -121,7 +125,7 @@ func buildMatrix(ctx context.Context, m expr.Value) ([]MatrixCombo, bool, error) | |||||
| 121 | return nil, false, fmt.Errorf("matrix vector '%s' does not contain any values", p.Key) | 125 | return nil, false, fmt.Errorf("matrix vector '%s' does not contain any values", p.Key) | ||
| 122 | } | 126 | } | ||
| 123 | if arr.Len() > MaxMatrixProduct { | 127 | if arr.Len() > MaxMatrixProduct { | ||
| 124 | - | return nil, false, errTooBig | 128 | + | return nil, false, errProductTooBig |
| 125 | } | 129 | } | ||
| 126 | vecs = append(vecs, vector{key: p.Key, items: arr.Items()}) | 130 | vecs = append(vecs, vector{key: p.Key, items: arr.Items()}) | ||
| 127 | } | 131 | } | ||
| @@ -153,7 +157,32 @@ type message string | |||||
| 153 | 157 | ||||
| 154 | func (m message) Error() string { return string(m) } | 158 | func (m message) Error() string { return string(m) } | ||
| 155 | 159 | ||||
| 156 | - | var errTooBig = fmt.Errorf("matrix size limit of %d exceeded", MaxMatrixJobs) | 160 | + | // errProductTooBig is returned when the cross product of the vectors is too |
| 161 | + | // large to be enumerated. GitHub would count the configurations left after | |||
| 162 | + | // the exclusions; this limit keeps the work bounded, see [MaxMatrixProduct]. | |||
| 163 | + | var errProductTooBig = fmt.Errorf("matrix too large: the cross product of the vectors exceeds %d combinations", MaxMatrixProduct) | |||
| 164 | + | ||||
| 165 | + | // configurationLimitError is returned when a matrix produces more than | |||
| 166 | + | // [MaxMatrixJobs] configurations. Its text is the one observed on the | |||
| 167 | + | // github.com run page for matrix-over-256 (not in the recordings, which only | |||
| 168 | + | // hold the failed run): "Strategy for job 'too-many' produced 272 | |||
| 169 | + | // configurations which exceeds the maximum of 256 configurations". Job is | |||
| 170 | + | // empty when the matrix was expanded without a job, see | |||
| 171 | + | // [Job.EvaluateStrategy]. | |||
| 172 | + | type configurationLimitError struct { | |||
| 173 | + | // Job is the id of the job. | |||
| 174 | + | Job string | |||
| 175 | + | // Count is the number of configurations the matrix produces. | |||
| 176 | + | Count int | |||
| 177 | + | } | |||
| 178 | + | ||||
| 179 | + | func (e *configurationLimitError) Error() string { | |||
| 180 | + | subject := "Strategy" | |||
| 181 | + | if e.Job != "" { | |||
| 182 | + | subject = fmt.Sprintf("Strategy for job '%s'", e.Job) | |||
| 183 | + | } | |||
| 184 | + | return fmt.Sprintf("%s produced %d configurations which exceeds the maximum of %d configurations", subject, e.Count, MaxMatrixJobs) | |||
| 185 | + | } | |||
| 157 | 186 | ||||
| 158 | type vector struct { | 187 | type vector struct { | ||
| 159 | key string | 188 | key string | ||
| @@ -188,19 +217,27 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 188 | // Exclude entries become lists of conditions on item positions, so that | 217 | // Exclude entries become lists of conditions on item positions, so that | ||
| 189 | // testing a combination costs a few array reads. | 218 | // testing a combination costs a few array reads. | ||
| 190 | var excl [][]condition | 219 | var excl [][]condition | ||
| 220 | + | var bad []error | |||
| 191 | for _, ex := range excludes { | 221 | for _, ex := range excludes { | ||
| 192 | var conds []condition | 222 | var conds []condition | ||
| 223 | + | unknown := false | |||
| 193 | for _, p := range ex.Pairs() { | 224 | for _, p := range ex.Pairs() { | ||
| 194 | vi, ok := index[strings.ToLower(p.Key)] | 225 | vi, ok := index[strings.ToLower(p.Key)] | ||
| 195 | if !ok { | 226 | if !ok { | ||
| 196 | - | return nil, false, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key)) | 227 | + | // One error per offending entry, for its first unknown key. |
| 228 | + | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | |||
| 229 | + | unknown = true | |||
| 230 | + | break | |||
| 197 | } | 231 | } | ||
| 198 | conds = append(conds, compileCondition(vecs, vi, p.Value)) | 232 | conds = append(conds, compileCondition(vecs, vi, p.Value)) | ||
| 199 | } | 233 | } | ||
| 200 | - | if len(conds) > 0 { | 234 | + | if len(conds) > 0 && !unknown { |
| 201 | excl = append(excl, conds) | 235 | excl = append(excl, conds) | ||
| 202 | } | 236 | } | ||
| 203 | } | 237 | } | ||
| 238 | + | if len(bad) > 0 { | |||
| 239 | + | return nil, false, errors.Join(bad...) | |||
| 240 | + | } | |||
| 204 | incl := make([]includeEntry, len(includes)) | 241 | incl := make([]includeEntry, len(includes)) | ||
| 205 | for n, inc := range includes { | 242 | for n, inc := range includes { | ||
| 206 | for _, p := range inc.Pairs() { | 243 | for _, p := range inc.Pairs() { | ||
| @@ -212,14 +249,17 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 212 | } | 249 | } | ||
| 213 | } | 250 | } | ||
| 214 | 251 | ||||
| 252 | + | // total counts every configuration, so that the error of a matrix over the | |||
| 253 | + | // limit tells how many it produces; only the first MaxMatrixJobs are built. | |||
| 215 | var combos []MatrixCombo | 254 | var combos []MatrixCombo | ||
| 255 | + | total := 0 | |||
| 216 | matched := make([]bool, len(includes)) | 256 | matched := make([]bool, len(includes)) | ||
| 217 | if len(vecs) > 0 { | 257 | if len(vecs) > 0 { | ||
| 218 | product := 1 | 258 | product := 1 | ||
| 219 | for _, v := range vecs { | 259 | for _, v := range vecs { | ||
| 220 | product *= len(v.items) | 260 | product *= len(v.items) | ||
| 221 | if product > MaxMatrixProduct { | 261 | if product > MaxMatrixProduct { | ||
| 222 | - | return nil, false, errTooBig | 262 | + | return nil, false, errProductTooBig |
| 223 | } | 263 | } | ||
| 224 | } | 264 | } | ||
| 225 | tuple := make([]int, len(vecs)) | 265 | tuple := make([]int, len(vecs)) | ||
| @@ -245,22 +285,28 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 245 | continue next | 285 | continue next | ||
| 246 | } | 286 | } | ||
| 247 | } | 287 | } | ||
| 248 | - | b := &comboBuilder{vals: map[string]expr.Value{}} | 288 | + | total++ |
| 249 | - | for k, v := range vecs { | 289 | + | build := total <= MaxMatrixJobs |
| 250 | - | b.set(v.key, v.items[tuple[k]]) | 290 | + | var b *comboBuilder |
| 291 | + | if build { | |||
| 292 | + | b = &comboBuilder{vals: map[string]expr.Value{}} | |||
| 293 | + | for k, v := range vecs { | |||
| 294 | + | b.set(v.key, v.items[tuple[k]]) | |||
| 295 | + | } | |||
| 251 | } | 296 | } | ||
| 252 | for n, inc := range incl { | 297 | for n, inc := range incl { | ||
| 253 | if !allHold(inc.conds, tuple) { | 298 | if !allHold(inc.conds, tuple) { | ||
| 254 | continue | 299 | continue | ||
| 255 | } | 300 | } | ||
| 256 | matched[n] = true | 301 | matched[n] = true | ||
| 257 | - | for _, p := range inc.extra { | 302 | + | if build { |
| 258 | - | b.set(p.Key, p.Value) | 303 | + | for _, p := range inc.extra { |
| 304 | + | b.set(p.Key, p.Value) | |||
| 305 | + | } | |||
| 259 | } | 306 | } | ||
| 260 | } | 307 | } | ||
| 261 | - | combos = append(combos, b.combo()) | 308 | + | if build { |
| 262 | - | if len(combos) > MaxMatrixJobs { | 309 | + | combos = append(combos, b.combo()) |
| 263 | - | return nil, false, errTooBig | |||
| 264 | } | 310 | } | ||
| 265 | } | 311 | } | ||
| 266 | } | 312 | } | ||
| @@ -268,14 +314,18 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 268 | if matched[n] { | 314 | if matched[n] { | ||
| 269 | continue | 315 | continue | ||
| 270 | } | 316 | } | ||
| 317 | + | total++ | |||
| 318 | + | if total > MaxMatrixJobs { | |||
| 319 | + | continue | |||
| 320 | + | } | |||
| 271 | b := &comboBuilder{vals: map[string]expr.Value{}} | 321 | b := &comboBuilder{vals: map[string]expr.Value{}} | ||
| 272 | for _, p := range inc.Pairs() { | 322 | for _, p := range inc.Pairs() { | ||
| 273 | b.set(p.Key, p.Value) | 323 | b.set(p.Key, p.Value) | ||
| 274 | } | 324 | } | ||
| 275 | combos = append(combos, b.combo()) | 325 | combos = append(combos, b.combo()) | ||
| 276 | - | if len(combos) > MaxMatrixJobs { | 326 | + | } |
| 277 | - | return nil, false, errTooBig | 327 | + | if total > MaxMatrixJobs { |
| 278 | - | } | 328 | + | return nil, false, &configurationLimitError{Count: total} |
| 279 | } | 329 | } | ||
| 280 | if len(combos) == 0 { | 330 | if len(combos) == 0 { | ||
| 281 | if len(vecs) == 0 && len(includes) == 0 && len(excludes) == 0 { | 331 | if len(vecs) == 0 && len(includes) == 0 && len(excludes) == 0 { | ||
| @@ -368,12 +418,16 @@ func valuesMatch(have, want expr.Value) bool { | |||||
| 368 | return false | 418 | return false | ||
| 369 | } | 419 | } | ||
| 370 | 420 | ||||
| 371 | - | // segment is the text a matrix value adds to the name of a job; arrays and | 421 | + | // segment is the text a matrix value adds to the name of a job. Null and the |
| 372 | - | // mappings add nothing. | 422 | + | // empty string add nothing, numbers and booleans are shown as in an |
| 423 | + | // expression ("1.5", "true"); all recorded (matrix-edge-values: | |||
| 424 | + | // "default-name", "two-axes (x)"). Arrays and mappings add nothing: assumed, | |||
| 425 | + | // not recorded. | |||
| 373 | func segment(v expr.Value) (string, bool) { | 426 | func segment(v expr.Value) (string, bool) { | ||
| 374 | switch v.(type) { | 427 | switch v.(type) { | ||
| 375 | - | case *expr.Array, *expr.Dictionary: | 428 | + | case nil, expr.Null, *expr.Array, *expr.Dictionary: |
| 376 | return "", false | 429 | return "", false | ||
| 377 | } | 430 | } | ||
| 378 | - | return expr.CoerceString(v), true | 431 | + | s := expr.CoerceString(v) |
| 432 | + | return s, s != "" | |||
| 379 | } | 433 | } | ||
M workflow/matrix_test.go +88 -13
| @@ -140,7 +140,7 @@ func TestExpandMatrix(t *testing.T) { | |||
| 140 | 140 | { | |
| 141 | 141 | "booleans, null and numbers in names", | |
| 142 | 142 | `{"matrix": {"a": [true, null, 1.5]}}`, | |
| 143 | - | []string{`a=true | job (true)`, `a=null | job ()`, `a=1.5 | job (1.5)`}, | |
| 143 | + | []string{`a=true | job (true)`, `a=null | job`, `a=1.5 | job (1.5)`}, | |
| 144 | 144 | }, | |
| 145 | 145 | { | |
| 146 | 146 | "objects and arrays are not in names", | |
| @@ -212,7 +212,7 @@ func TestMatrixLimit(t *testing.T) { | |||
| 212 | 212 | t.Errorf("256 jobs: %d, %v", len(c), err) | |
| 213 | 213 | } | |
| 214 | 214 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(16)+`, "b": `+vec(16)+`, "include": [{"a": 99}]}}`)) | |
| 215 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit of 256 exceeded") { | |
| 215 | + | if err == nil || !strings.Contains(err.Error(), "Strategy produced 257 configurations which exceeds the maximum of 256 configurations") { | |
| 216 | 216 | t.Errorf("257 jobs: %v", err) | |
| 217 | 217 | } | |
| 218 | 218 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(17)+`, "b": `+vec(16)+`}}`)) | |
| @@ -290,7 +290,7 @@ func TestHostileMatrixFailsFast(t *testing.T) { | |||
| 290 | 290 | src := `{"matrix": {"a": ` + vec(100) + `, "b": ` + vec(100) + `, "c": ` + vec(100) + `, "exclude": ` + strings.ReplaceAll(entries(256), `"a": 1`, `"c": 5`) + `}}` | |
| 291 | 291 | start := time.Now() | |
| 292 | 292 | _, err := ExpandMatrix(ctxbg, jsonValue(t, src)) | |
| 293 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | |
| 293 | + | if err == nil || !strings.Contains(err.Error(), "matrix too large") { | |
| 294 | 294 | t.Errorf("err = %v", err) | |
| 295 | 295 | } | |
| 296 | 296 | if d := time.Since(start); d > time.Second { | |
| @@ -321,11 +321,12 @@ func TestMatrixProductCap(t *testing.T) { | |||
| 321 | 321 | } | |
| 322 | 322 | // 2^16 is allowed by the cap (and fails on the job limit), 2^16+1 is refused outright. | |
| 323 | 323 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct/2)+`, "b": `+vec(2)+`, "c": `+vec(1)+`}}`)) | |
| 324 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | |
| 324 | + | var limit *configurationLimitError | |
| 325 | + | if !errors.As(err, &limit) || limit.Count != MaxMatrixProduct { | |
| 325 | 326 | t.Errorf("err = %v", err) | |
| 326 | 327 | } | |
| 327 | 328 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct+1)+`}}`)) | |
| 328 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | |
| 329 | + | if err == nil || !strings.Contains(err.Error(), "matrix too large") { | |
| 329 | 330 | t.Errorf("one huge vector: %v", err) | |
| 330 | 331 | } | |
| 331 | 332 | } | |
| @@ -348,15 +349,89 @@ func TestMatrixContextIsHonoured(t *testing.T) { | |||
| 348 | 349 | } | |
| 349 | 350 | } | |
| 350 | 351 | ||
| 351 | - | func TestNullMatrixValueSegment(t *testing.T) { | |
| 352 | - | // TODO(verify with recording): a null value shows as an empty segment, | |
| 353 | - | // "job (, 1)"; GitHub may skip it. | |
| 354 | - | combos, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [null], "b": [1]}}`)) | |
| 355 | - | if err != nil { | |
| 356 | - | t.Fatal(err) | |
| 352 | + | // TestRecordedMatrixNames follows the job names GitHub gave in the scenarios | |
| 353 | + | // matrix-edge-values (b-null-values.yml) and matrix-job-name. | |
| 354 | + | func TestRecordedMatrixNames(t *testing.T) { | |
| 355 | + | names := func(matrix string) []string { | |
| 356 | + | t.Helper() | |
| 357 | + | combos, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": `+matrix+`}`)) | |
| 358 | + | if err != nil { | |
| 359 | + | t.Fatal(err) | |
| 360 | + | } | |
| 361 | + | var out []string | |
| 362 | + | for _, c := range combos { | |
| 363 | + | out = append(out, c.DisplayName("default-name")) | |
| 364 | + | } | |
| 365 | + | return out | |
| 366 | + | } | |
| 367 | + | // v: [1, null, '', true, 1.50, 'x y'] gave default-name, default-name, | |
| 368 | + | // default-name (1), default-name (1.5), default-name (true) and | |
| 369 | + | // default-name (x y): null and '' add nothing, 1.50 is 1.5. | |
| 370 | + | want := []string{"default-name (1)", "default-name", "default-name", "default-name (true)", "default-name (1.5)", "default-name (x y)"} | |
| 371 | + | if got := names(`{"v": [1, null, "", true, 1.50, "x y"]}`); !reflect.DeepEqual(got, want) { | |
| 372 | + | t.Errorf("one axis: got %q, want %q", got, want) | |
| 373 | + | } | |
| 374 | + | // a: [x, null], b: ['', y] gave "two-axes (x)", "two-axes (x, y)", | |
| 375 | + | // "two-axes (y)" and "two-axes". | |
| 376 | + | want = []string{"default-name (x)", "default-name (x, y)", "default-name", "default-name (y)"} | |
| 377 | + | if got := names(`{"a": ["x", null], "b": ["", "y"]}`); !reflect.DeepEqual(got, want) { | |
| 378 | + | t.Errorf("two axes: got %q, want %q", got, want) | |
| 379 | + | } | |
| 380 | + | // matrix-job-name: os [a, b] x n [1, 2] gave "default-name (a, 1)" ... | |
| 381 | + | want = []string{"default-name (a, 1)", "default-name (a, 2)", "default-name (b, 1)", "default-name (b, 2)"} | |
| 382 | + | if got := names(`{"os": ["a", "b"], "n": [1, 2]}`); !reflect.DeepEqual(got, want) { | |
| 383 | + | t.Errorf("matrix-job-name: got %q, want %q", got, want) | |
| 384 | + | } | |
| 385 | + | // Objects are not listed. Assumed, not recorded: matrix-value-types has | |
| 386 | + | // custom names only. | |
| 387 | + | if got := names(`{"n": [1], "obj": [{"k": "v"}]}`); !reflect.DeepEqual(got, []string{"default-name (1)"}) { | |
| 388 | + | t.Errorf("object value: got %q", got) | |
| 357 | 389 | } | |
| 358 | - | if got := combos[0].DisplayName("job"); got != "job (, 1)" { | |
| 359 | - | t.Errorf("name = %q", got) | |
| 390 | + | } | |
| 391 | + | ||
| 392 | + | // TestRecordedExcludeUnknownKey is matrix-edge-values a-exclude-unknown-key.yml: | |
| 393 | + | // exclude: [{nokey: x}, {os: one, nokey: y}] made the workflow invalid. The | |
| 394 | + | // recording holds only the failed run; the message, once per offending entry, | |
| 395 | + | // was observed on the github.com run page. | |
| 396 | + | func TestRecordedExcludeUnknownKey(t *testing.T) { | |
| 397 | + | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"nokey": "x"}, {"os": "one", "nokey": "y"}]}}`)) | |
| 398 | + | const one = "Matrix exclude key 'nokey' does not match any key within the matrix" | |
| 399 | + | if err == nil || err.Error() != one+"\n"+one { | |
| 400 | + | t.Fatalf("err = %v, want the message once per offending entry", err) | |
| 401 | + | } | |
| 402 | + | // An entry that is fine does not add one. | |
| 403 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | |
| 404 | + | if err == nil || err.Error() != one { | |
| 405 | + | t.Errorf("err = %v, want %q", err, one) | |
| 406 | + | } | |
| 407 | + | } | |
| 408 | + | ||
| 409 | + | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | |
| 410 | + | // configurations in job too-many made the workflow invalid. The recording | |
| 411 | + | // holds only the failed run; the message was observed on the github.com run | |
| 412 | + | // page. | |
| 413 | + | func TestRecordedMatrixOver256(t *testing.T) { | |
| 414 | + | w, errs := Parse("ci.yml", []byte(`on: push | |
| 415 | + | jobs: | |
| 416 | + | too-many: | |
| 417 | + | runs-on: x | |
| 418 | + | strategy: | |
| 419 | + | matrix: | |
| 420 | + | a: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17] | |
| 421 | + | b: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16] | |
| 422 | + | steps: [{run: a}] | |
| 423 | + | `)) | |
| 424 | + | if len(errs) > 0 { | |
| 425 | + | t.Fatal(errs) | |
| 426 | + | } | |
| 427 | + | _, err := w.Job("too-many").EvaluateStrategy(ctxbg, template.Env{}) | |
| 428 | + | const want = "Strategy for job 'too-many' produced 272 configurations which exceeds the maximum of 256 configurations" | |
| 429 | + | if err == nil || err.Error() != want { | |
| 430 | + | t.Errorf("err = %v, want %q", err, want) | |
| 431 | + | } | |
| 432 | + | var limit *configurationLimitError | |
| 433 | + | if !errors.As(err, &limit) || limit.Job != "too-many" || limit.Count != 272 { | |
| 434 | + | t.Errorf("err = %#v", err) | |
| 360 | 435 | } | |
| 361 | 436 | } | |
| 362 | 437 | ||
| @@ -140,7 +140,7 @@ func TestExpandMatrix(t *testing.T) { | |||||
| 140 | { | 140 | { | ||
| 141 | "booleans, null and numbers in names", | 141 | "booleans, null and numbers in names", | ||
| 142 | `{"matrix": {"a": [true, null, 1.5]}}`, | 142 | `{"matrix": {"a": [true, null, 1.5]}}`, | ||
| 143 | - | []string{`a=true | job (true)`, `a=null | job ()`, `a=1.5 | job (1.5)`}, | 143 | + | []string{`a=true | job (true)`, `a=null | job`, `a=1.5 | job (1.5)`}, |
| 144 | }, | 144 | }, | ||
| 145 | { | 145 | { | ||
| 146 | "objects and arrays are not in names", | 146 | "objects and arrays are not in names", | ||
| @@ -212,7 +212,7 @@ func TestMatrixLimit(t *testing.T) { | |||||
| 212 | t.Errorf("256 jobs: %d, %v", len(c), err) | 212 | t.Errorf("256 jobs: %d, %v", len(c), err) | ||
| 213 | } | 213 | } | ||
| 214 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(16)+`, "b": `+vec(16)+`, "include": [{"a": 99}]}}`)) | 214 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(16)+`, "b": `+vec(16)+`, "include": [{"a": 99}]}}`)) | ||
| 215 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit of 256 exceeded") { | 215 | + | if err == nil || !strings.Contains(err.Error(), "Strategy produced 257 configurations which exceeds the maximum of 256 configurations") { |
| 216 | t.Errorf("257 jobs: %v", err) | 216 | t.Errorf("257 jobs: %v", err) | ||
| 217 | } | 217 | } | ||
| 218 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(17)+`, "b": `+vec(16)+`}}`)) | 218 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(17)+`, "b": `+vec(16)+`}}`)) | ||
| @@ -290,7 +290,7 @@ func TestHostileMatrixFailsFast(t *testing.T) { | |||||
| 290 | src := `{"matrix": {"a": ` + vec(100) + `, "b": ` + vec(100) + `, "c": ` + vec(100) + `, "exclude": ` + strings.ReplaceAll(entries(256), `"a": 1`, `"c": 5`) + `}}` | 290 | src := `{"matrix": {"a": ` + vec(100) + `, "b": ` + vec(100) + `, "c": ` + vec(100) + `, "exclude": ` + strings.ReplaceAll(entries(256), `"a": 1`, `"c": 5`) + `}}` | ||
| 291 | start := time.Now() | 291 | start := time.Now() | ||
| 292 | _, err := ExpandMatrix(ctxbg, jsonValue(t, src)) | 292 | _, err := ExpandMatrix(ctxbg, jsonValue(t, src)) | ||
| 293 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | 293 | + | if err == nil || !strings.Contains(err.Error(), "matrix too large") { |
| 294 | t.Errorf("err = %v", err) | 294 | t.Errorf("err = %v", err) | ||
| 295 | } | 295 | } | ||
| 296 | if d := time.Since(start); d > time.Second { | 296 | if d := time.Since(start); d > time.Second { | ||
| @@ -321,11 +321,12 @@ func TestMatrixProductCap(t *testing.T) { | |||||
| 321 | } | 321 | } | ||
| 322 | // 2^16 is allowed by the cap (and fails on the job limit), 2^16+1 is refused outright. | 322 | // 2^16 is allowed by the cap (and fails on the job limit), 2^16+1 is refused outright. | ||
| 323 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct/2)+`, "b": `+vec(2)+`, "c": `+vec(1)+`}}`)) | 323 | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct/2)+`, "b": `+vec(2)+`, "c": `+vec(1)+`}}`)) | ||
| 324 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | 324 | + | var limit *configurationLimitError |
| 325 | + | if !errors.As(err, &limit) || limit.Count != MaxMatrixProduct { | |||
| 325 | t.Errorf("err = %v", err) | 326 | t.Errorf("err = %v", err) | ||
| 326 | } | 327 | } | ||
| 327 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct+1)+`}}`)) | 328 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": `+vec(MaxMatrixProduct+1)+`}}`)) | ||
| 328 | - | if err == nil || !strings.Contains(err.Error(), "matrix size limit") { | 329 | + | if err == nil || !strings.Contains(err.Error(), "matrix too large") { |
| 329 | t.Errorf("one huge vector: %v", err) | 330 | t.Errorf("one huge vector: %v", err) | ||
| 330 | } | 331 | } | ||
| 331 | } | 332 | } | ||
| @@ -348,15 +349,89 @@ func TestMatrixContextIsHonoured(t *testing.T) { | |||||
| 348 | } | 349 | } | ||
| 349 | } | 350 | } | ||
| 350 | 351 | ||||
| 351 | - | func TestNullMatrixValueSegment(t *testing.T) { | 352 | + | // TestRecordedMatrixNames follows the job names GitHub gave in the scenarios |
| 352 | - | // TODO(verify with recording): a null value shows as an empty segment, | 353 | + | // matrix-edge-values (b-null-values.yml) and matrix-job-name. |
| 353 | - | // "job (, 1)"; GitHub may skip it. | 354 | + | func TestRecordedMatrixNames(t *testing.T) { |
| 354 | - | combos, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [null], "b": [1]}}`)) | 355 | + | names := func(matrix string) []string { |
| 355 | - | if err != nil { | 356 | + | t.Helper() |
| 356 | - | t.Fatal(err) | 357 | + | combos, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": `+matrix+`}`)) |
| 358 | + | if err != nil { | |||
| 359 | + | t.Fatal(err) | |||
| 360 | + | } | |||
| 361 | + | var out []string | |||
| 362 | + | for _, c := range combos { | |||
| 363 | + | out = append(out, c.DisplayName("default-name")) | |||
| 364 | + | } | |||
| 365 | + | return out | |||
| 366 | + | } | |||
| 367 | + | // v: [1, null, '', true, 1.50, 'x y'] gave default-name, default-name, | |||
| 368 | + | // default-name (1), default-name (1.5), default-name (true) and | |||
| 369 | + | // default-name (x y): null and '' add nothing, 1.50 is 1.5. | |||
| 370 | + | want := []string{"default-name (1)", "default-name", "default-name", "default-name (true)", "default-name (1.5)", "default-name (x y)"} | |||
| 371 | + | if got := names(`{"v": [1, null, "", true, 1.50, "x y"]}`); !reflect.DeepEqual(got, want) { | |||
| 372 | + | t.Errorf("one axis: got %q, want %q", got, want) | |||
| 373 | + | } | |||
| 374 | + | // a: [x, null], b: ['', y] gave "two-axes (x)", "two-axes (x, y)", | |||
| 375 | + | // "two-axes (y)" and "two-axes". | |||
| 376 | + | want = []string{"default-name (x)", "default-name (x, y)", "default-name", "default-name (y)"} | |||
| 377 | + | if got := names(`{"a": ["x", null], "b": ["", "y"]}`); !reflect.DeepEqual(got, want) { | |||
| 378 | + | t.Errorf("two axes: got %q, want %q", got, want) | |||
| 379 | + | } | |||
| 380 | + | // matrix-job-name: os [a, b] x n [1, 2] gave "default-name (a, 1)" ... | |||
| 381 | + | want = []string{"default-name (a, 1)", "default-name (a, 2)", "default-name (b, 1)", "default-name (b, 2)"} | |||
| 382 | + | if got := names(`{"os": ["a", "b"], "n": [1, 2]}`); !reflect.DeepEqual(got, want) { | |||
| 383 | + | t.Errorf("matrix-job-name: got %q, want %q", got, want) | |||
| 384 | + | } | |||
| 385 | + | // Objects are not listed. Assumed, not recorded: matrix-value-types has | |||
| 386 | + | // custom names only. | |||
| 387 | + | if got := names(`{"n": [1], "obj": [{"k": "v"}]}`); !reflect.DeepEqual(got, []string{"default-name (1)"}) { | |||
| 388 | + | t.Errorf("object value: got %q", got) | |||
| 357 | } | 389 | } | ||
| 358 | - | if got := combos[0].DisplayName("job"); got != "job (, 1)" { | 390 | + | } |
| 359 | - | t.Errorf("name = %q", got) | 391 | + | |
| 392 | + | // TestRecordedExcludeUnknownKey is matrix-edge-values a-exclude-unknown-key.yml: | |||
| 393 | + | // exclude: [{nokey: x}, {os: one, nokey: y}] made the workflow invalid. The | |||
| 394 | + | // recording holds only the failed run; the message, once per offending entry, | |||
| 395 | + | // was observed on the github.com run page. | |||
| 396 | + | func TestRecordedExcludeUnknownKey(t *testing.T) { | |||
| 397 | + | _, err := ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"nokey": "x"}, {"os": "one", "nokey": "y"}]}}`)) | |||
| 398 | + | const one = "Matrix exclude key 'nokey' does not match any key within the matrix" | |||
| 399 | + | if err == nil || err.Error() != one+"\n"+one { | |||
| 400 | + | t.Fatalf("err = %v, want the message once per offending entry", err) | |||
| 401 | + | } | |||
| 402 | + | // An entry that is fine does not add one. | |||
| 403 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | |||
| 404 | + | if err == nil || err.Error() != one { | |||
| 405 | + | t.Errorf("err = %v, want %q", err, one) | |||
| 406 | + | } | |||
| 407 | + | } | |||
| 408 | + | ||||
| 409 | + | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | |||
| 410 | + | // configurations in job too-many made the workflow invalid. The recording | |||
| 411 | + | // holds only the failed run; the message was observed on the github.com run | |||
| 412 | + | // page. | |||
| 413 | + | func TestRecordedMatrixOver256(t *testing.T) { | |||
| 414 | + | w, errs := Parse("ci.yml", []byte(`on: push | |||
| 415 | + | jobs: | |||
| 416 | + | too-many: | |||
| 417 | + | runs-on: x | |||
| 418 | + | strategy: | |||
| 419 | + | matrix: | |||
| 420 | + | a: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17] | |||
| 421 | + | b: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16] | |||
| 422 | + | steps: [{run: a}] | |||
| 423 | + | `)) | |||
| 424 | + | if len(errs) > 0 { | |||
| 425 | + | t.Fatal(errs) | |||
| 426 | + | } | |||
| 427 | + | _, err := w.Job("too-many").EvaluateStrategy(ctxbg, template.Env{}) | |||
| 428 | + | const want = "Strategy for job 'too-many' produced 272 configurations which exceeds the maximum of 256 configurations" | |||
| 429 | + | if err == nil || err.Error() != want { | |||
| 430 | + | t.Errorf("err = %v, want %q", err, want) | |||
| 431 | + | } | |||
| 432 | + | var limit *configurationLimitError | |||
| 433 | + | if !errors.As(err, &limit) || limit.Job != "too-many" || limit.Count != 272 { | |||
| 434 | + | t.Errorf("err = %#v", err) | |||
| 360 | } | 435 | } | ||
| 361 | } | 436 | } | ||
| 362 | 437 | ||||
M workflow/push.go +14 -8
| @@ -6,9 +6,13 @@ import "strings" | |||
| 6 | 6 | ||
| 7 | 7 | // Limits GitHub applies to push events. | |
| 8 | 8 | const ( | |
| 9 | - | // MaxFilesEvaluated is the number of changed files a paths filter looks | |
| 10 | - | // at; files after the 300th are ignored. | |
| 11 | - | MaxFilesEvaluated = 300 | |
| 9 | + | // MaxFilesEvaluated is the number of changed files a paths or paths-ignore | |
| 10 | + | // filter looks at; files after the 3000th are ignored. Recorded on | |
| 11 | + | // github.com (filter-paths-3000-boundary): a push of exactly 3000 files | |
| 12 | + | // whose only match is the last one runs the workflow, the same push with | |
| 13 | + | // 3001 files does not. It is not 300 (filter-paths-300-files and | |
| 14 | + | // filter-paths-1200-files: a match beyond the 300th file still runs). | |
| 15 | + | MaxFilesEvaluated = 3000 | |
| 12 | 16 | // MaxTagsPerPush is the number of tags that can be pushed at once before | |
| 13 | 17 | // GitHub creates no event for them. | |
| 14 | 18 | MaxTagsPerPush = 3 | |
| @@ -43,15 +47,17 @@ type PushInfo struct { | |||
| 43 | 47 | // - when both a branches and a tags filter exist, a ref is checked against | |
| 44 | 48 | // the filter for its kind; | |
| 45 | 49 | // - paths and paths-ignore apply to branch pushes only, against the first | |
| 46 | - | // 300 changed files; when the list is truncated they pass; | |
| 50 | + | // 3000 changed files; when the list is unavailable (see | |
| 51 | + | // [PushInfo.FilesTruncated]) they pass; | |
| 47 | 52 | // - patterns are evaluated in order and the last one that matches wins. | |
| 48 | 53 | // | |
| 49 | 54 | // Commit-message directives are separate: see [SkipCI]. | |
| 50 | 55 | // | |
| 51 | - | // TODO(verify with recording): the order in which a branch filter and a | |
| 52 | - | // paths filter combine is "both must pass" (documented); behaviour on refs | |
| 53 | - | // that are neither a branch nor a tag, and on a push that changes no file, is | |
| 54 | - | // assumed. | |
| 56 | + | // Recorded on github.com: push-tags-limit, filter-tags, filter-branches-ignore, | |
| 57 | + | // filter-tag-branches-only, filter-paths-tag, push-delete-branch and | |
| 58 | + | // push-delete-tag, see TestMatchPushRecordedTags. Still assumed: the | |
| 59 | + | // behaviour on refs that are neither a branch nor a tag, and on a push that | |
| 60 | + | // changes no file. | |
| 55 | 61 | func (w *Workflow) MatchPush(p PushInfo) bool { | |
| 56 | 62 | if w == nil || w.On == nil { | |
| 57 | 63 | return false | |
| @@ -6,9 +6,13 @@ import "strings" | |||||
| 6 | 6 | ||||
| 7 | // Limits GitHub applies to push events. | 7 | // Limits GitHub applies to push events. | ||
| 8 | const ( | 8 | const ( | ||
| 9 | - | // MaxFilesEvaluated is the number of changed files a paths filter looks | 9 | + | // MaxFilesEvaluated is the number of changed files a paths or paths-ignore |
| 10 | - | // at; files after the 300th are ignored. | 10 | + | // filter looks at; files after the 3000th are ignored. Recorded on |
| 11 | - | MaxFilesEvaluated = 300 | 11 | + | // github.com (filter-paths-3000-boundary): a push of exactly 3000 files |
| 12 | + | // whose only match is the last one runs the workflow, the same push with | |||
| 13 | + | // 3001 files does not. It is not 300 (filter-paths-300-files and | |||
| 14 | + | // filter-paths-1200-files: a match beyond the 300th file still runs). | |||
| 15 | + | MaxFilesEvaluated = 3000 | |||
| 12 | // MaxTagsPerPush is the number of tags that can be pushed at once before | 16 | // MaxTagsPerPush is the number of tags that can be pushed at once before | ||
| 13 | // GitHub creates no event for them. | 17 | // GitHub creates no event for them. | ||
| 14 | MaxTagsPerPush = 3 | 18 | MaxTagsPerPush = 3 | ||
| @@ -43,15 +47,17 @@ type PushInfo struct { | |||||
| 43 | // - when both a branches and a tags filter exist, a ref is checked against | 47 | // - when both a branches and a tags filter exist, a ref is checked against | ||
| 44 | // the filter for its kind; | 48 | // the filter for its kind; | ||
| 45 | // - paths and paths-ignore apply to branch pushes only, against the first | 49 | // - paths and paths-ignore apply to branch pushes only, against the first | ||
| 46 | - | // 300 changed files; when the list is truncated they pass; | 50 | + | // 3000 changed files; when the list is unavailable (see |
| 51 | + | // [PushInfo.FilesTruncated]) they pass; | |||
| 47 | // - patterns are evaluated in order and the last one that matches wins. | 52 | // - patterns are evaluated in order and the last one that matches wins. | ||
| 48 | // | 53 | // | ||
| 49 | // Commit-message directives are separate: see [SkipCI]. | 54 | // Commit-message directives are separate: see [SkipCI]. | ||
| 50 | // | 55 | // | ||
| 51 | - | // TODO(verify with recording): the order in which a branch filter and a | 56 | + | // Recorded on github.com: push-tags-limit, filter-tags, filter-branches-ignore, |
| 52 | - | // paths filter combine is "both must pass" (documented); behaviour on refs | 57 | + | // filter-tag-branches-only, filter-paths-tag, push-delete-branch and |
| 53 | - | // that are neither a branch nor a tag, and on a push that changes no file, is | 58 | + | // push-delete-tag, see TestMatchPushRecordedTags. Still assumed: the |
| 54 | - | // assumed. | 59 | + | // behaviour on refs that are neither a branch nor a tag, and on a push that |
| 60 | + | // changes no file. | |||
| 55 | func (w *Workflow) MatchPush(p PushInfo) bool { | 61 | func (w *Workflow) MatchPush(p PushInfo) bool { | ||
| 56 | if w == nil || w.On == nil { | 62 | if w == nil || w.On == nil { | ||
| 57 | return false | 63 | return false | ||
M workflow/push_test.go +209 -24
| @@ -93,30 +93,215 @@ func TestMatchPush(t *testing.T) { | |||
| 93 | 93 | } | |
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | - | func TestMatchPushFirst300Files(t *testing.T) { | |
| 97 | - | w := pushWorkflow(t, "on:\n push:\n paths: ['src/**']") | |
| 98 | - | files := make([]string, 0, 301) | |
| 99 | - | for i := 0; i < MaxFilesEvaluated; i++ { | |
| 100 | - | files = append(files, fmt.Sprintf("docs/%d.md", i)) | |
| 101 | - | } | |
| 102 | - | files = append(files, "src/late.go") // the 301st | |
| 103 | - | if w.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | |
| 104 | - | t.Error("a match beyond the 300th file must not run the workflow") | |
| 105 | - | } | |
| 106 | - | files[299] = "src/early.go" | |
| 107 | - | if !w.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | |
| 108 | - | t.Error("a match among the first 300 files must run the workflow") | |
| 109 | - | } | |
| 110 | - | wi := pushWorkflow(t, "on:\n push:\n paths-ignore: ['docs/**']") | |
| 111 | - | files = files[:0] | |
| 112 | - | for i := 0; i < MaxFilesEvaluated; i++ { | |
| 113 | - | files = append(files, fmt.Sprintf("docs/%d.md", i)) | |
| 114 | - | } | |
| 115 | - | files = append(files, "src/late.go") | |
| 116 | - | // TODO(verify with recording): with paths-ignore, files beyond the 300th | |
| 117 | - | // are not considered either, so everything seen is ignored. | |
| 118 | - | if wi.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | |
| 119 | - | t.Error("paths-ignore: only the first 300 files are evaluated") | |
| 96 | + | // numberedFiles returns n paths "dir/f-00000.txt" ... in the order of the diff, | |
| 97 | + | // which for GitHub is sorted by path. | |
| 98 | + | func numberedFiles(dir string, n int) []string { | |
| 99 | + | files := make([]string, 0, n+1) | |
| 100 | + | for i := 0; i < n; i++ { | |
| 101 | + | files = append(files, fmt.Sprintf("%s/f-%05d.txt", dir, i)) | |
| 102 | + | } | |
| 103 | + | return files | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | // TestMatchPushFileLimit follows the recordings filter-paths-300-files, | |
| 107 | + | // filter-paths-1200-files, filter-paths-3000-boundary and | |
| 108 | + | // filter-paths-3100-files: a paths or paths-ignore filter looks at the first | |
| 109 | + | // 3000 changed files, whose only match (or only file not ignored) sorts last. | |
| 110 | + | func TestMatchPushFileLimit(t *testing.T) { | |
| 111 | + | if MaxFilesEvaluated != 3000 { | |
| 112 | + | t.Fatalf("MaxFilesEvaluated = %d, want 3000", MaxFilesEvaluated) | |
| 113 | + | } | |
| 114 | + | paths := pushWorkflow(t, "on:\n push:\n paths: ['zzz/**']") | |
| 115 | + | early := pushWorkflow(t, "on:\n push:\n paths: ['gen/**']") | |
| 116 | + | ignore := pushWorkflow(t, "on:\n push:\n paths-ignore: ['gen/**']") | |
| 117 | + | for _, tc := range []struct { | |
| 118 | + | scenario string | |
| 119 | + | total int // changed files, the last one is zzz/late.txt | |
| 120 | + | late bool | |
| 121 | + | early bool | |
| 122 | + | ignore bool | |
| 123 | + | }{ | |
| 124 | + | // filter-paths-300-files, push 2 (301 files): late-match, early-match | |
| 125 | + | // and ignore-late all ran. | |
| 126 | + | {"filter-paths-300-files", 301, true, true, true}, | |
| 127 | + | // filter-paths-1200-files, push 2 (1201 files): same. | |
| 128 | + | {"filter-paths-1200-files", 1201, true, true, true}, | |
| 129 | + | // filter-paths-3000-boundary, push 2 (3000 files, match last): ran. | |
| 130 | + | {"filter-paths-3000-boundary 3000", 3000, true, true, true}, | |
| 131 | + | // filter-paths-3000-boundary, push 3 (3001 files): the match is the | |
| 132 | + | // 3001st file, the workflow did not run. | |
| 133 | + | {"filter-paths-3000-boundary 3001", 3001, false, true, false}, | |
| 134 | + | // filter-paths-3100-files, push 2 (3101 files): late-match and | |
| 135 | + | // ignore-late did not run, early-match did. | |
| 136 | + | {"filter-paths-3100-files", 3101, false, true, false}, | |
| 137 | + | } { | |
| 138 | + | files := append(numberedFiles("gen", tc.total-1), "zzz/late.txt") | |
| 139 | + | p := PushInfo{Ref: "refs/heads/scn/x", ChangedFiles: files} | |
| 140 | + | if got := paths.MatchPush(p); got != tc.late { | |
| 141 | + | t.Errorf("%s: paths with the match last = %v, want %v", tc.scenario, got, tc.late) | |
| 142 | + | } | |
| 143 | + | if got := early.MatchPush(p); got != tc.early { | |
| 144 | + | t.Errorf("%s: paths with an early match = %v, want %v", tc.scenario, got, tc.early) | |
| 145 | + | } | |
| 146 | + | if got := ignore.MatchPush(p); got != tc.ignore { | |
| 147 | + | t.Errorf("%s: paths-ignore with the last file not ignored = %v, want %v", tc.scenario, got, tc.ignore) | |
| 148 | + | } | |
| 149 | + | } | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | // TestMatchPushRecordedTags replays what github.com did in the scenarios | |
| 153 | + | // push-tags-limit, filter-tags, filter-branches-ignore, | |
| 154 | + | // filter-tag-branches-only and filter-paths-tag. The branches are named | |
| 155 | + | // scn/<scenario>/<run> and the tags scn-<scenario>-<run>-<n>. | |
| 156 | + | func TestMatchPushRecordedTags(t *testing.T) { | |
| 157 | + | tag := func(n, count int) PushInfo { | |
| 158 | + | return PushInfo{Ref: fmt.Sprintf("refs/tags/scn-filter-tags-1-%d", n), Created: true, TagCount: count} | |
| 159 | + | } | |
| 160 | + | branch := PushInfo{Ref: "refs/heads/scn/filter-tags/1", Created: true, ChangedFiles: []string{"a.txt"}} | |
| 161 | + | tests := []struct { | |
| 162 | + | scenario string | |
| 163 | + | on string | |
| 164 | + | push PushInfo | |
| 165 | + | want bool | |
| 166 | + | }{ | |
| 167 | + | // push-tags-limit: three tags at once start three runs, four tags none. | |
| 168 | + | {"push-tags-limit 3 tags", "on: push", tag(1, 3), true}, | |
| 169 | + | {"push-tags-limit 3 tags (third)", "on: push", tag(3, 3), true}, | |
| 170 | + | {"push-tags-limit 4 tags (first)", "on: push", tag(1, 4), false}, | |
| 171 | + | {"push-tags-limit 4 tags (last)", "on: push", tag(4, 4), false}, | |
| 172 | + | {"push-tags-limit 4 tags, tags filter", "on:\n push:\n tags: ['scn-*']", tag(2, 4), false}, | |
| 173 | + | ||
| 174 | + | // filter-tags, push 1 (a branch): t-both and t-branches-only ran, the | |
| 175 | + | // tags-only workflows did not. | |
| 176 | + | {"filter-tags branch t-both", "on:\n push:\n branches: ['scn/**']\n tags: ['scn-*']", branch, true}, | |
| 177 | + | {"filter-tags branch t-branches-only", "on:\n push:\n branches: ['scn/**']", branch, true}, | |
| 178 | + | {"filter-tags branch t-match", "on:\n push:\n tags: ['scn-*']", branch, false}, | |
| 179 | + | {"filter-tags branch t-ignore-other", "on:\n push:\n tags-ignore: ['v*']", branch, false}, | |
| 180 | + | {"filter-tags branch t-negated", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-1'", branch, false}, | |
| 181 | + | // filter-tags, push 2 (tag scn-...-1): ran for t-both, t-match, | |
| 182 | + | // t-ignore-other and t-negated-other; not for t-branches-only, t-ignore, | |
| 183 | + | // t-negated and t-nomatch. | |
| 184 | + | {"filter-tags tag t-both", "on:\n push:\n branches: ['scn/**']\n tags: ['scn-*']", tag(1, 1), true}, | |
| 185 | + | {"filter-tags tag t-match", "on:\n push:\n tags: ['scn-*']", tag(1, 1), true}, | |
| 186 | + | {"filter-tags tag t-ignore-other", "on:\n push:\n tags-ignore: ['v*']", tag(1, 1), true}, | |
| 187 | + | {"filter-tags tag t-negated-other", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-2'", tag(1, 1), true}, | |
| 188 | + | {"filter-tags tag t-branches-only", "on:\n push:\n branches: ['scn/**']", tag(1, 1), false}, | |
| 189 | + | {"filter-tags tag t-ignore", "on:\n push:\n tags-ignore: ['scn-*']", tag(1, 1), false}, | |
| 190 | + | {"filter-tags tag t-negated", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-1'", tag(1, 1), false}, | |
| 191 | + | {"filter-tags tag t-nomatch", "on:\n push:\n tags: ['v*']", tag(1, 1), false}, | |
| 192 | + | ||
| 193 | + | // filter-branches-ignore: the branch push ran bi-other and | |
| 194 | + | // bi-single-star, not bi-ignored and not bi-tags-too; the tag push ran | |
| 195 | + | // only bi-tags-too (branches-ignore alone ignores tags). | |
| 196 | + | {"filter-branches-ignore branch bi-ignored", "on:\n push:\n branches-ignore: ['scn/**']", branch, false}, | |
| 197 | + | {"filter-branches-ignore branch bi-other", "on:\n push:\n branches-ignore: ['main', 'release/**']", branch, true}, | |
| 198 | + | {"filter-branches-ignore branch bi-single-star", "on:\n push:\n branches-ignore: ['scn/*']", branch, true}, | |
| 199 | + | {"filter-branches-ignore branch bi-tags-too", "on:\n push:\n branches-ignore: ['scn/**']\n tags: ['scn-*']", branch, false}, | |
| 200 | + | {"filter-branches-ignore tag bi-tags-too", "on:\n push:\n branches-ignore: ['scn/**']\n tags: ['scn-*']", tag(1, 1), true}, | |
| 201 | + | {"filter-branches-ignore tag bi-ignored", "on:\n push:\n branches-ignore: ['scn/**']", tag(1, 1), false}, | |
| 202 | + | {"filter-branches-ignore tag bi-other", "on:\n push:\n branches-ignore: ['main']", tag(1, 1), false}, | |
| 203 | + | ||
| 204 | + | // filter-tag-branches-only: a tag pushed to a repository whose only | |
| 205 | + | // workflow filters on branches starts nothing. | |
| 206 | + | {"filter-tag-branches-only", "on:\n push:\n branches: ['scn/**']", tag(1, 1), false}, | |
| 207 | + | ||
| 208 | + | // filter-paths-tag: paths are not evaluated for tags. | |
| 209 | + | {"filter-paths-tag paths-only", "on:\n push:\n paths: ['nothing/**']", tag(1, 1), true}, | |
| 210 | + | {"filter-paths-tag tag-and-paths-ignore", "on:\n push:\n tags: ['scn-*']\n paths-ignore: ['**']", tag(1, 1), true}, | |
| 211 | + | {"filter-paths-tag tag-and-paths-nomatch", "on:\n push:\n tags: ['scn-*']\n paths: ['nothing/**']", tag(1, 1), true}, | |
| 212 | + | {"filter-paths-tag branches-and-paths", "on:\n push:\n branches: ['scn/**']\n paths: ['nothing/**']", tag(1, 1), false}, | |
| 213 | + | {"filter-paths-tag branches-and-paths, branch", "on:\n push:\n branches: ['scn/**']\n paths: ['nothing/**']", branch, false}, | |
| 214 | + | ||
| 215 | + | // push-delete-branch, push-delete-tag: a deletion starts nothing, | |
| 216 | + | // whatever the filters. | |
| 217 | + | {"push-delete-branch", "on: push", PushInfo{Ref: "refs/heads/scn/push-delete-branch/1", Deleted: true}, false}, | |
| 218 | + | {"push-delete-tag", "on: push", PushInfo{Ref: "refs/tags/scn-push-delete-tag-1-1", Deleted: true, TagCount: 1}, false}, | |
| 219 | + | } | |
| 220 | + | for _, tc := range tests { | |
| 221 | + | t.Run(tc.scenario, func(t *testing.T) { | |
| 222 | + | if got := pushWorkflow(t, tc.on).MatchPush(tc.push); got != tc.want { | |
| 223 | + | t.Errorf("MatchPush = %v, want %v", got, tc.want) | |
| 224 | + | } | |
| 225 | + | }) | |
| 226 | + | } | |
| 227 | + | } | |
| 228 | + | ||
| 229 | + | // TestMatchPushRecordedGlobs replays filter-branches (branch | |
| 230 | + | // scn/filter-branches/<run>: ran for b-match, b-negated-then-readded, | |
| 231 | + | // b-prefix-only, b-star-star and b-two-levels; not for b-negated, b-one-level | |
| 232 | + | // and b-other), filter-paths and filter-paths-ignore. | |
| 233 | + | func TestMatchPushRecordedGlobs(t *testing.T) { | |
| 234 | + | br := PushInfo{Ref: "refs/heads/scn/filter-branches/1"} | |
| 235 | + | for _, tc := range []struct { | |
| 236 | + | name, filter string | |
| 237 | + | want bool | |
| 238 | + | }{ | |
| 239 | + | {"b-match", "branches: ['scn/filter-branches/**']", true}, | |
| 240 | + | {"b-negated-then-readded", "branches: ['**', '!scn/**', 'scn/filter-branches/**']", true}, | |
| 241 | + | {"b-prefix-only", "branches: ['scn/filter-*/**']", true}, | |
| 242 | + | {"b-star-star", "branches: ['**']", true}, | |
| 243 | + | {"b-two-levels", "branches: ['scn/*/*']", true}, | |
| 244 | + | {"b-negated", "branches: ['scn/**', '!scn/filter-branches/**']", false}, | |
| 245 | + | {"b-one-level", "branches: ['scn/*']", false}, | |
| 246 | + | {"b-other", "branches: [main, 'release/**']", false}, | |
| 247 | + | } { | |
| 248 | + | if got := pushWorkflow(t, "on:\n push:\n "+tc.filter).MatchPush(br); got != tc.want { | |
| 249 | + | t.Errorf("filter-branches %s: %v, want %v", tc.name, got, tc.want) | |
| 250 | + | } | |
| 251 | + | } | |
| 252 | + | ||
| 253 | + | // filter-paths: the files of each push. | |
| 254 | + | pushes := [][]string{ | |
| 255 | + | {".github/workflows/p-docs.yml", "docs/readme.md", "other/x.txt", "src/a.txt", "top.md"}, // 1: the workflow files are new too | |
| 256 | + | {"docs/readme.md"}, // 2 | |
| 257 | + | {"src/a.txt"}, // 3 | |
| 258 | + | {"top.md"}, // 4 | |
| 259 | + | {"docs/new.md", "other/x.txt"}, // 5 | |
| 260 | + | } | |
| 261 | + | for _, tc := range []struct { | |
| 262 | + | name, filter string | |
| 263 | + | ran [5]bool | |
| 264 | + | }{ | |
| 265 | + | {"p-docs", "paths: ['docs/**']", [5]bool{true, true, false, false, true}}, | |
| 266 | + | {"p-double-star-md", "paths: ['**.md']", [5]bool{true, true, false, true, true}}, | |
| 267 | + | {"p-exact-file", "paths: ['src/a.txt']", [5]bool{true, false, true, false, false}}, | |
| 268 | + | {"p-negated", "paths: ['**', '!docs/**']", [5]bool{true, false, true, true, true}}, | |
| 269 | + | {"p-no-match", "paths: ['nothing/**']", [5]bool{}}, | |
| 270 | + | {"p-single-star-md", "paths: ['*.md']", [5]bool{true, false, false, true, false}}, | |
| 271 | + | {"p-src", "paths: ['src/**']", [5]bool{true, false, true, false, false}}, | |
| 272 | + | } { | |
| 273 | + | w := pushWorkflow(t, "on:\n push:\n branches: ['scn/**']\n "+tc.filter) | |
| 274 | + | for n, files := range pushes { | |
| 275 | + | got := w.MatchPush(PushInfo{Ref: "refs/heads/scn/filter-paths/1", ChangedFiles: files}) | |
| 276 | + | if got != tc.ran[n] { | |
| 277 | + | t.Errorf("filter-paths %s push %d: %v, want %v", tc.name, n+1, got, tc.ran[n]) | |
| 278 | + | } | |
| 279 | + | } | |
| 280 | + | } | |
| 281 | + | ||
| 282 | + | // filter-paths-ignore: pushes 1 to 4 changed these files. | |
| 283 | + | ignorePushes := [][]string{ | |
| 284 | + | {".github/workflows/pi-docs.yml", "docs/readme.md", "src/a.txt"}, // 1: the workflow files are new too | |
| 285 | + | {"docs/readme.md"}, // 2 | |
| 286 | + | {"docs/readme.md", "src/a.txt"}, // 3 | |
| 287 | + | {"src/a.txt"}, // 4 | |
| 288 | + | } | |
| 289 | + | for _, tc := range []struct { | |
| 290 | + | name, filter string | |
| 291 | + | ran [4]bool | |
| 292 | + | }{ | |
| 293 | + | {"pi-docs", "paths-ignore: ['docs/**']", [4]bool{true, false, true, true}}, | |
| 294 | + | {"pi-md", "paths-ignore: ['**.md']", [4]bool{true, false, true, true}}, | |
| 295 | + | {"pi-src-docs", "paths-ignore: ['docs/**', 'src/**']", [4]bool{true, false, false, false}}, | |
| 296 | + | {"pi-everything", "paths-ignore: ['**']", [4]bool{}}, | |
| 297 | + | } { | |
| 298 | + | w := pushWorkflow(t, "on:\n push:\n branches: ['scn/**']\n "+tc.filter) | |
| 299 | + | for n, files := range ignorePushes { | |
| 300 | + | got := w.MatchPush(PushInfo{Ref: "refs/heads/scn/filter-paths-ignore/1", ChangedFiles: files}) | |
| 301 | + | if got != tc.ran[n] { | |
| 302 | + | t.Errorf("filter-paths-ignore %s push %d: %v, want %v", tc.name, n+1, got, tc.ran[n]) | |
| 303 | + | } | |
| 304 | + | } | |
| 120 | 305 | } | |
| 121 | 306 | } | |
| 122 | 307 | ||
| @@ -93,30 +93,215 @@ func TestMatchPush(t *testing.T) { | |||||
| 93 | } | 93 | } | ||
| 94 | } | 94 | } | ||
| 95 | 95 | ||||
| 96 | - | func TestMatchPushFirst300Files(t *testing.T) { | 96 | + | // numberedFiles returns n paths "dir/f-00000.txt" ... in the order of the diff, |
| 97 | - | w := pushWorkflow(t, "on:\n push:\n paths: ['src/**']") | 97 | + | // which for GitHub is sorted by path. |
| 98 | - | files := make([]string, 0, 301) | 98 | + | func numberedFiles(dir string, n int) []string { |
| 99 | - | for i := 0; i < MaxFilesEvaluated; i++ { | 99 | + | files := make([]string, 0, n+1) |
| 100 | - | files = append(files, fmt.Sprintf("docs/%d.md", i)) | 100 | + | for i := 0; i < n; i++ { |
| 101 | - | } | 101 | + | files = append(files, fmt.Sprintf("%s/f-%05d.txt", dir, i)) |
| 102 | - | files = append(files, "src/late.go") // the 301st | 102 | + | } |
| 103 | - | if w.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | 103 | + | return files |
| 104 | - | t.Error("a match beyond the 300th file must not run the workflow") | 104 | + | } |
| 105 | - | } | 105 | + | |
| 106 | - | files[299] = "src/early.go" | 106 | + | // TestMatchPushFileLimit follows the recordings filter-paths-300-files, |
| 107 | - | if !w.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | 107 | + | // filter-paths-1200-files, filter-paths-3000-boundary and |
| 108 | - | t.Error("a match among the first 300 files must run the workflow") | 108 | + | // filter-paths-3100-files: a paths or paths-ignore filter looks at the first |
| 109 | - | } | 109 | + | // 3000 changed files, whose only match (or only file not ignored) sorts last. |
| 110 | - | wi := pushWorkflow(t, "on:\n push:\n paths-ignore: ['docs/**']") | 110 | + | func TestMatchPushFileLimit(t *testing.T) { |
| 111 | - | files = files[:0] | 111 | + | if MaxFilesEvaluated != 3000 { |
| 112 | - | for i := 0; i < MaxFilesEvaluated; i++ { | 112 | + | t.Fatalf("MaxFilesEvaluated = %d, want 3000", MaxFilesEvaluated) |
| 113 | - | files = append(files, fmt.Sprintf("docs/%d.md", i)) | 113 | + | } |
| 114 | - | } | 114 | + | paths := pushWorkflow(t, "on:\n push:\n paths: ['zzz/**']") |
| 115 | - | files = append(files, "src/late.go") | 115 | + | early := pushWorkflow(t, "on:\n push:\n paths: ['gen/**']") |
| 116 | - | // TODO(verify with recording): with paths-ignore, files beyond the 300th | 116 | + | ignore := pushWorkflow(t, "on:\n push:\n paths-ignore: ['gen/**']") |
| 117 | - | // are not considered either, so everything seen is ignored. | 117 | + | for _, tc := range []struct { |
| 118 | - | if wi.MatchPush(PushInfo{Ref: "refs/heads/main", ChangedFiles: files}) { | 118 | + | scenario string |
| 119 | - | t.Error("paths-ignore: only the first 300 files are evaluated") | 119 | + | total int // changed files, the last one is zzz/late.txt |
| 120 | + | late bool | |||
| 121 | + | early bool | |||
| 122 | + | ignore bool | |||
| 123 | + | }{ | |||
| 124 | + | // filter-paths-300-files, push 2 (301 files): late-match, early-match | |||
| 125 | + | // and ignore-late all ran. | |||
| 126 | + | {"filter-paths-300-files", 301, true, true, true}, | |||
| 127 | + | // filter-paths-1200-files, push 2 (1201 files): same. | |||
| 128 | + | {"filter-paths-1200-files", 1201, true, true, true}, | |||
| 129 | + | // filter-paths-3000-boundary, push 2 (3000 files, match last): ran. | |||
| 130 | + | {"filter-paths-3000-boundary 3000", 3000, true, true, true}, | |||
| 131 | + | // filter-paths-3000-boundary, push 3 (3001 files): the match is the | |||
| 132 | + | // 3001st file, the workflow did not run. | |||
| 133 | + | {"filter-paths-3000-boundary 3001", 3001, false, true, false}, | |||
| 134 | + | // filter-paths-3100-files, push 2 (3101 files): late-match and | |||
| 135 | + | // ignore-late did not run, early-match did. | |||
| 136 | + | {"filter-paths-3100-files", 3101, false, true, false}, | |||
| 137 | + | } { | |||
| 138 | + | files := append(numberedFiles("gen", tc.total-1), "zzz/late.txt") | |||
| 139 | + | p := PushInfo{Ref: "refs/heads/scn/x", ChangedFiles: files} | |||
| 140 | + | if got := paths.MatchPush(p); got != tc.late { | |||
| 141 | + | t.Errorf("%s: paths with the match last = %v, want %v", tc.scenario, got, tc.late) | |||
| 142 | + | } | |||
| 143 | + | if got := early.MatchPush(p); got != tc.early { | |||
| 144 | + | t.Errorf("%s: paths with an early match = %v, want %v", tc.scenario, got, tc.early) | |||
| 145 | + | } | |||
| 146 | + | if got := ignore.MatchPush(p); got != tc.ignore { | |||
| 147 | + | t.Errorf("%s: paths-ignore with the last file not ignored = %v, want %v", tc.scenario, got, tc.ignore) | |||
| 148 | + | } | |||
| 149 | + | } | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | // TestMatchPushRecordedTags replays what github.com did in the scenarios | |||
| 153 | + | // push-tags-limit, filter-tags, filter-branches-ignore, | |||
| 154 | + | // filter-tag-branches-only and filter-paths-tag. The branches are named | |||
| 155 | + | // scn/<scenario>/<run> and the tags scn-<scenario>-<run>-<n>. | |||
| 156 | + | func TestMatchPushRecordedTags(t *testing.T) { | |||
| 157 | + | tag := func(n, count int) PushInfo { | |||
| 158 | + | return PushInfo{Ref: fmt.Sprintf("refs/tags/scn-filter-tags-1-%d", n), Created: true, TagCount: count} | |||
| 159 | + | } | |||
| 160 | + | branch := PushInfo{Ref: "refs/heads/scn/filter-tags/1", Created: true, ChangedFiles: []string{"a.txt"}} | |||
| 161 | + | tests := []struct { | |||
| 162 | + | scenario string | |||
| 163 | + | on string | |||
| 164 | + | push PushInfo | |||
| 165 | + | want bool | |||
| 166 | + | }{ | |||
| 167 | + | // push-tags-limit: three tags at once start three runs, four tags none. | |||
| 168 | + | {"push-tags-limit 3 tags", "on: push", tag(1, 3), true}, | |||
| 169 | + | {"push-tags-limit 3 tags (third)", "on: push", tag(3, 3), true}, | |||
| 170 | + | {"push-tags-limit 4 tags (first)", "on: push", tag(1, 4), false}, | |||
| 171 | + | {"push-tags-limit 4 tags (last)", "on: push", tag(4, 4), false}, | |||
| 172 | + | {"push-tags-limit 4 tags, tags filter", "on:\n push:\n tags: ['scn-*']", tag(2, 4), false}, | |||
| 173 | + | ||||
| 174 | + | // filter-tags, push 1 (a branch): t-both and t-branches-only ran, the | |||
| 175 | + | // tags-only workflows did not. | |||
| 176 | + | {"filter-tags branch t-both", "on:\n push:\n branches: ['scn/**']\n tags: ['scn-*']", branch, true}, | |||
| 177 | + | {"filter-tags branch t-branches-only", "on:\n push:\n branches: ['scn/**']", branch, true}, | |||
| 178 | + | {"filter-tags branch t-match", "on:\n push:\n tags: ['scn-*']", branch, false}, | |||
| 179 | + | {"filter-tags branch t-ignore-other", "on:\n push:\n tags-ignore: ['v*']", branch, false}, | |||
| 180 | + | {"filter-tags branch t-negated", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-1'", branch, false}, | |||
| 181 | + | // filter-tags, push 2 (tag scn-...-1): ran for t-both, t-match, | |||
| 182 | + | // t-ignore-other and t-negated-other; not for t-branches-only, t-ignore, | |||
| 183 | + | // t-negated and t-nomatch. | |||
| 184 | + | {"filter-tags tag t-both", "on:\n push:\n branches: ['scn/**']\n tags: ['scn-*']", tag(1, 1), true}, | |||
| 185 | + | {"filter-tags tag t-match", "on:\n push:\n tags: ['scn-*']", tag(1, 1), true}, | |||
| 186 | + | {"filter-tags tag t-ignore-other", "on:\n push:\n tags-ignore: ['v*']", tag(1, 1), true}, | |||
| 187 | + | {"filter-tags tag t-negated-other", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-2'", tag(1, 1), true}, | |||
| 188 | + | {"filter-tags tag t-branches-only", "on:\n push:\n branches: ['scn/**']", tag(1, 1), false}, | |||
| 189 | + | {"filter-tags tag t-ignore", "on:\n push:\n tags-ignore: ['scn-*']", tag(1, 1), false}, | |||
| 190 | + | {"filter-tags tag t-negated", "on:\n push:\n tags:\n - 'scn-*'\n - '!scn-*-1'", tag(1, 1), false}, | |||
| 191 | + | {"filter-tags tag t-nomatch", "on:\n push:\n tags: ['v*']", tag(1, 1), false}, | |||
| 192 | + | ||||
| 193 | + | // filter-branches-ignore: the branch push ran bi-other and | |||
| 194 | + | // bi-single-star, not bi-ignored and not bi-tags-too; the tag push ran | |||
| 195 | + | // only bi-tags-too (branches-ignore alone ignores tags). | |||
| 196 | + | {"filter-branches-ignore branch bi-ignored", "on:\n push:\n branches-ignore: ['scn/**']", branch, false}, | |||
| 197 | + | {"filter-branches-ignore branch bi-other", "on:\n push:\n branches-ignore: ['main', 'release/**']", branch, true}, | |||
| 198 | + | {"filter-branches-ignore branch bi-single-star", "on:\n push:\n branches-ignore: ['scn/*']", branch, true}, | |||
| 199 | + | {"filter-branches-ignore branch bi-tags-too", "on:\n push:\n branches-ignore: ['scn/**']\n tags: ['scn-*']", branch, false}, | |||
| 200 | + | {"filter-branches-ignore tag bi-tags-too", "on:\n push:\n branches-ignore: ['scn/**']\n tags: ['scn-*']", tag(1, 1), true}, | |||
| 201 | + | {"filter-branches-ignore tag bi-ignored", "on:\n push:\n branches-ignore: ['scn/**']", tag(1, 1), false}, | |||
| 202 | + | {"filter-branches-ignore tag bi-other", "on:\n push:\n branches-ignore: ['main']", tag(1, 1), false}, | |||
| 203 | + | ||||
| 204 | + | // filter-tag-branches-only: a tag pushed to a repository whose only | |||
| 205 | + | // workflow filters on branches starts nothing. | |||
| 206 | + | {"filter-tag-branches-only", "on:\n push:\n branches: ['scn/**']", tag(1, 1), false}, | |||
| 207 | + | ||||
| 208 | + | // filter-paths-tag: paths are not evaluated for tags. | |||
| 209 | + | {"filter-paths-tag paths-only", "on:\n push:\n paths: ['nothing/**']", tag(1, 1), true}, | |||
| 210 | + | {"filter-paths-tag tag-and-paths-ignore", "on:\n push:\n tags: ['scn-*']\n paths-ignore: ['**']", tag(1, 1), true}, | |||
| 211 | + | {"filter-paths-tag tag-and-paths-nomatch", "on:\n push:\n tags: ['scn-*']\n paths: ['nothing/**']", tag(1, 1), true}, | |||
| 212 | + | {"filter-paths-tag branches-and-paths", "on:\n push:\n branches: ['scn/**']\n paths: ['nothing/**']", tag(1, 1), false}, | |||
| 213 | + | {"filter-paths-tag branches-and-paths, branch", "on:\n push:\n branches: ['scn/**']\n paths: ['nothing/**']", branch, false}, | |||
| 214 | + | ||||
| 215 | + | // push-delete-branch, push-delete-tag: a deletion starts nothing, | |||
| 216 | + | // whatever the filters. | |||
| 217 | + | {"push-delete-branch", "on: push", PushInfo{Ref: "refs/heads/scn/push-delete-branch/1", Deleted: true}, false}, | |||
| 218 | + | {"push-delete-tag", "on: push", PushInfo{Ref: "refs/tags/scn-push-delete-tag-1-1", Deleted: true, TagCount: 1}, false}, | |||
| 219 | + | } | |||
| 220 | + | for _, tc := range tests { | |||
| 221 | + | t.Run(tc.scenario, func(t *testing.T) { | |||
| 222 | + | if got := pushWorkflow(t, tc.on).MatchPush(tc.push); got != tc.want { | |||
| 223 | + | t.Errorf("MatchPush = %v, want %v", got, tc.want) | |||
| 224 | + | } | |||
| 225 | + | }) | |||
| 226 | + | } | |||
| 227 | + | } | |||
| 228 | + | ||||
| 229 | + | // TestMatchPushRecordedGlobs replays filter-branches (branch | |||
| 230 | + | // scn/filter-branches/<run>: ran for b-match, b-negated-then-readded, | |||
| 231 | + | // b-prefix-only, b-star-star and b-two-levels; not for b-negated, b-one-level | |||
| 232 | + | // and b-other), filter-paths and filter-paths-ignore. | |||
| 233 | + | func TestMatchPushRecordedGlobs(t *testing.T) { | |||
| 234 | + | br := PushInfo{Ref: "refs/heads/scn/filter-branches/1"} | |||
| 235 | + | for _, tc := range []struct { | |||
| 236 | + | name, filter string | |||
| 237 | + | want bool | |||
| 238 | + | }{ | |||
| 239 | + | {"b-match", "branches: ['scn/filter-branches/**']", true}, | |||
| 240 | + | {"b-negated-then-readded", "branches: ['**', '!scn/**', 'scn/filter-branches/**']", true}, | |||
| 241 | + | {"b-prefix-only", "branches: ['scn/filter-*/**']", true}, | |||
| 242 | + | {"b-star-star", "branches: ['**']", true}, | |||
| 243 | + | {"b-two-levels", "branches: ['scn/*/*']", true}, | |||
| 244 | + | {"b-negated", "branches: ['scn/**', '!scn/filter-branches/**']", false}, | |||
| 245 | + | {"b-one-level", "branches: ['scn/*']", false}, | |||
| 246 | + | {"b-other", "branches: [main, 'release/**']", false}, | |||
| 247 | + | } { | |||
| 248 | + | if got := pushWorkflow(t, "on:\n push:\n "+tc.filter).MatchPush(br); got != tc.want { | |||
| 249 | + | t.Errorf("filter-branches %s: %v, want %v", tc.name, got, tc.want) | |||
| 250 | + | } | |||
| 251 | + | } | |||
| 252 | + | ||||
| 253 | + | // filter-paths: the files of each push. | |||
| 254 | + | pushes := [][]string{ | |||
| 255 | + | {".github/workflows/p-docs.yml", "docs/readme.md", "other/x.txt", "src/a.txt", "top.md"}, // 1: the workflow files are new too | |||
| 256 | + | {"docs/readme.md"}, // 2 | |||
| 257 | + | {"src/a.txt"}, // 3 | |||
| 258 | + | {"top.md"}, // 4 | |||
| 259 | + | {"docs/new.md", "other/x.txt"}, // 5 | |||
| 260 | + | } | |||
| 261 | + | for _, tc := range []struct { | |||
| 262 | + | name, filter string | |||
| 263 | + | ran [5]bool | |||
| 264 | + | }{ | |||
| 265 | + | {"p-docs", "paths: ['docs/**']", [5]bool{true, true, false, false, true}}, | |||
| 266 | + | {"p-double-star-md", "paths: ['**.md']", [5]bool{true, true, false, true, true}}, | |||
| 267 | + | {"p-exact-file", "paths: ['src/a.txt']", [5]bool{true, false, true, false, false}}, | |||
| 268 | + | {"p-negated", "paths: ['**', '!docs/**']", [5]bool{true, false, true, true, true}}, | |||
| 269 | + | {"p-no-match", "paths: ['nothing/**']", [5]bool{}}, | |||
| 270 | + | {"p-single-star-md", "paths: ['*.md']", [5]bool{true, false, false, true, false}}, | |||
| 271 | + | {"p-src", "paths: ['src/**']", [5]bool{true, false, true, false, false}}, | |||
| 272 | + | } { | |||
| 273 | + | w := pushWorkflow(t, "on:\n push:\n branches: ['scn/**']\n "+tc.filter) | |||
| 274 | + | for n, files := range pushes { | |||
| 275 | + | got := w.MatchPush(PushInfo{Ref: "refs/heads/scn/filter-paths/1", ChangedFiles: files}) | |||
| 276 | + | if got != tc.ran[n] { | |||
| 277 | + | t.Errorf("filter-paths %s push %d: %v, want %v", tc.name, n+1, got, tc.ran[n]) | |||
| 278 | + | } | |||
| 279 | + | } | |||
| 280 | + | } | |||
| 281 | + | ||||
| 282 | + | // filter-paths-ignore: pushes 1 to 4 changed these files. | |||
| 283 | + | ignorePushes := [][]string{ | |||
| 284 | + | {".github/workflows/pi-docs.yml", "docs/readme.md", "src/a.txt"}, // 1: the workflow files are new too | |||
| 285 | + | {"docs/readme.md"}, // 2 | |||
| 286 | + | {"docs/readme.md", "src/a.txt"}, // 3 | |||
| 287 | + | {"src/a.txt"}, // 4 | |||
| 288 | + | } | |||
| 289 | + | for _, tc := range []struct { | |||
| 290 | + | name, filter string | |||
| 291 | + | ran [4]bool | |||
| 292 | + | }{ | |||
| 293 | + | {"pi-docs", "paths-ignore: ['docs/**']", [4]bool{true, false, true, true}}, | |||
| 294 | + | {"pi-md", "paths-ignore: ['**.md']", [4]bool{true, false, true, true}}, | |||
| 295 | + | {"pi-src-docs", "paths-ignore: ['docs/**', 'src/**']", [4]bool{true, false, false, false}}, | |||
| 296 | + | {"pi-everything", "paths-ignore: ['**']", [4]bool{}}, | |||
| 297 | + | } { | |||
| 298 | + | w := pushWorkflow(t, "on:\n push:\n branches: ['scn/**']\n "+tc.filter) | |||
| 299 | + | for n, files := range ignorePushes { | |||
| 300 | + | got := w.MatchPush(PushInfo{Ref: "refs/heads/scn/filter-paths-ignore/1", ChangedFiles: files}) | |||
| 301 | + | if got != tc.ran[n] { | |||
| 302 | + | t.Errorf("filter-paths-ignore %s push %d: %v, want %v", tc.name, n+1, got, tc.ran[n]) | |||
| 303 | + | } | |||
| 304 | + | } | |||
| 120 | } | 305 | } | ||
| 121 | } | 306 | } | ||
| 122 | 307 | ||||
A workflow/recorded_test.go +145 -0
| @@ -0,0 +1,145 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package workflow | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "reflect" | |
| 7 | + | "testing" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | // TestRecordedInvalidWorkflows parses the workflow files of the recorded | |
| 11 | + | // scenarios expr-status-in-env, expr-status-in-run, invalid-needs and | |
| 12 | + | // invalid-schema, which GitHub refused (a failed run without jobs; the | |
| 13 | + | // recordings do not hold the messages), and checks that this package refuses | |
| 14 | + | // them, with the reference parser's messages, except for the status functions | |
| 15 | + | // whose message was observed on the github.com run page. The valid sibling of | |
| 16 | + | // each scenario is in TestRecordedValid. | |
| 17 | + | func TestRecordedInvalidWorkflows(t *testing.T) { | |
| 18 | + | const on = "on:\n push:\n branches: ['scn/**']\n" | |
| 19 | + | tests := []struct { | |
| 20 | + | name, src string | |
| 21 | + | want []string | |
| 22 | + | }{ | |
| 23 | + | {"expr-status-in-env", on + `jobs: | |
| 24 | + | main: | |
| 25 | + | runs-on: ubuntu-24.04 | |
| 26 | + | steps: | |
| 27 | + | - name: status functions in env | |
| 28 | + | env: | |
| 29 | + | ST_SUCCESS: ${{ success() }} | |
| 30 | + | ST_FAILURE: ${{ failure() }} | |
| 31 | + | ST_ALWAYS: ${{ always() }} | |
| 32 | + | ST_CANCELLED: ${{ cancelled() }} | |
| 33 | + | run: env | grep '^ST_' | sort | |
| 34 | + | `, []string{ | |
| 35 | + | "w.yml (Line: 10, Col: 23): Unrecognized function: 'success'. Located at position 1 within expression: success()", | |
| 36 | + | "w.yml (Line: 11, Col: 23): Unrecognized function: 'failure'. Located at position 1 within expression: failure()", | |
| 37 | + | "w.yml (Line: 12, Col: 22): Unrecognized function: 'always'. Located at position 1 within expression: always()", | |
| 38 | + | "w.yml (Line: 13, Col: 25): Unrecognized function: 'cancelled'. Located at position 1 within expression: cancelled()", | |
| 39 | + | }}, | |
| 40 | + | {"expr-status-in-run", on + `jobs: | |
| 41 | + | main: | |
| 42 | + | runs-on: ubuntu-24.04 | |
| 43 | + | steps: | |
| 44 | + | - name: status functions in run | |
| 45 | + | run: | | |
| 46 | + | echo "success=${{ success() }}" | |
| 47 | + | echo "failure=${{ failure() }}" | |
| 48 | + | `, []string{ | |
| 49 | + | "w.yml (Line: 9, Col: 14): Unrecognized function: 'success'. Located at position 1 within expression: success()", | |
| 50 | + | "w.yml (Line: 9, Col: 14): Unrecognized function: 'failure'. Located at position 1 within expression: failure()", | |
| 51 | + | }}, | |
| 52 | + | {"invalid-needs unknown-need", on + `jobs: | |
| 53 | + | main: | |
| 54 | + | needs: nowhere | |
| 55 | + | runs-on: ubuntu-24.04 | |
| 56 | + | steps: | |
| 57 | + | - run: echo SHOULD-NOT-RUN | |
| 58 | + | `, []string{"w.yml (Line: 5, Col: 3): The workflow must contain at least one job with no dependencies."}}, | |
| 59 | + | {"invalid-needs unknown-need-with-root", on + `jobs: | |
| 60 | + | root: | |
| 61 | + | runs-on: ubuntu-24.04 | |
| 62 | + | steps: | |
| 63 | + | - run: echo SHOULD-NOT-RUN | |
| 64 | + | dependent: | |
| 65 | + | needs: [root, nowhere] | |
| 66 | + | runs-on: ubuntu-24.04 | |
| 67 | + | steps: | |
| 68 | + | - run: echo SHOULD-NOT-RUN | |
| 69 | + | `, []string{"w.yml (Line: 10, Col: 19): Job 'dependent' depends on unknown job 'nowhere'."}}, | |
| 70 | + | {"invalid-needs cycle", on + `jobs: | |
| 71 | + | a: | |
| 72 | + | needs: b | |
| 73 | + | runs-on: ubuntu-24.04 | |
| 74 | + | steps: | |
| 75 | + | - run: echo SHOULD-NOT-RUN | |
| 76 | + | b: | |
| 77 | + | needs: a | |
| 78 | + | runs-on: ubuntu-24.04 | |
| 79 | + | steps: | |
| 80 | + | - run: echo SHOULD-NOT-RUN | |
| 81 | + | `, []string{"w.yml (Line: 5, Col: 3): The workflow must contain at least one job with no dependencies."}}, | |
| 82 | + | {"invalid-needs cycle-with-root", on + `jobs: | |
| 83 | + | root: | |
| 84 | + | runs-on: ubuntu-24.04 | |
| 85 | + | steps: | |
| 86 | + | - run: echo SHOULD-NOT-RUN | |
| 87 | + | a: | |
| 88 | + | needs: [root, b] | |
| 89 | + | runs-on: ubuntu-24.04 | |
| 90 | + | steps: | |
| 91 | + | - run: echo SHOULD-NOT-RUN | |
| 92 | + | b: | |
| 93 | + | needs: a | |
| 94 | + | runs-on: ubuntu-24.04 | |
| 95 | + | steps: | |
| 96 | + | - run: echo SHOULD-NOT-RUN | |
| 97 | + | `, []string{ | |
| 98 | + | "w.yml (Line: 10, Col: 19): Job 'a' depends on job 'b' which creates a cycle in the dependency graph.", | |
| 99 | + | "w.yml (Line: 15, Col: 12): Job 'b' depends on job 'a' which creates a cycle in the dependency graph.", | |
| 100 | + | }}, | |
| 101 | + | {"invalid-schema unknown-key", on + `jobs: | |
| 102 | + | main: | |
| 103 | + | runs-on: ubuntu-24.04 | |
| 104 | + | unknown-key: true | |
| 105 | + | steps: | |
| 106 | + | - run: echo SHOULD-NOT-RUN | |
| 107 | + | `, []string{"w.yml (Line: 7, Col: 5): Unexpected value 'unknown-key'"}}, | |
| 108 | + | {"invalid-schema no-runs-on", on + `jobs: | |
| 109 | + | main: | |
| 110 | + | steps: | |
| 111 | + | - run: echo SHOULD-NOT-RUN | |
| 112 | + | `, []string{"w.yml (Line: 6, Col: 5): Required property is missing: runs-on"}}, | |
| 113 | + | {"invalid-schema empty-step", on + `jobs: | |
| 114 | + | main: | |
| 115 | + | runs-on: ubuntu-24.04 | |
| 116 | + | steps: | |
| 117 | + | - name: nothing to run | |
| 118 | + | `, []string{"w.yml (Line: 8, Col: 9): There's not enough info to determine what you meant. Add one of these properties: run, shell, uses, with, working-directory"}}, | |
| 119 | + | } | |
| 120 | + | for _, tc := range tests { | |
| 121 | + | t.Run(tc.name, func(t *testing.T) { | |
| 122 | + | _, errs := Parse("w.yml", []byte(tc.src)) | |
| 123 | + | var got []string | |
| 124 | + | for _, e := range errs { | |
| 125 | + | got = append(got, e.Error()) | |
| 126 | + | } | |
| 127 | + | if !reflect.DeepEqual(got, tc.want) { | |
| 128 | + | t.Errorf("errors:\n got %q\n want %q", got, tc.want) | |
| 129 | + | } | |
| 130 | + | }) | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | // TestRecordedValid checks that the valid siblings of the invalid scenarios | |
| 135 | + | // and the status functions in conditions, which GitHub accepted, parse. | |
| 136 | + | func TestRecordedValid(t *testing.T) { | |
| 137 | + | for name, src := range map[string]string{ | |
| 138 | + | "valid sibling": "name: valid-sibling\non:\n push:\n branches: ['scn/**']\n tags: ['scn-*']\njobs:\n main:\n runs-on: ubuntu-24.04\n steps:\n - run: echo valid-sibling-ran\n", | |
| 139 | + | "status functions in if": "on: push\njobs:\n main:\n runs-on: x\n if: success() || failure()\n steps:\n - if: always() && !cancelled()\n run: echo\n", | |
| 140 | + | } { | |
| 141 | + | if _, errs := Parse("w.yml", []byte(src)); len(errs) > 0 { | |
| 142 | + | t.Errorf("%s: %v", name, errs) | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | } | |
| @@ -0,0 +1,145 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package workflow | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "reflect" | |||
| 7 | + | "testing" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | // TestRecordedInvalidWorkflows parses the workflow files of the recorded | |||
| 11 | + | // scenarios expr-status-in-env, expr-status-in-run, invalid-needs and | |||
| 12 | + | // invalid-schema, which GitHub refused (a failed run without jobs; the | |||
| 13 | + | // recordings do not hold the messages), and checks that this package refuses | |||
| 14 | + | // them, with the reference parser's messages, except for the status functions | |||
| 15 | + | // whose message was observed on the github.com run page. The valid sibling of | |||
| 16 | + | // each scenario is in TestRecordedValid. | |||
| 17 | + | func TestRecordedInvalidWorkflows(t *testing.T) { | |||
| 18 | + | const on = "on:\n push:\n branches: ['scn/**']\n" | |||
| 19 | + | tests := []struct { | |||
| 20 | + | name, src string | |||
| 21 | + | want []string | |||
| 22 | + | }{ | |||
| 23 | + | {"expr-status-in-env", on + `jobs: | |||
| 24 | + | main: | |||
| 25 | + | runs-on: ubuntu-24.04 | |||
| 26 | + | steps: | |||
| 27 | + | - name: status functions in env | |||
| 28 | + | env: | |||
| 29 | + | ST_SUCCESS: ${{ success() }} | |||
| 30 | + | ST_FAILURE: ${{ failure() }} | |||
| 31 | + | ST_ALWAYS: ${{ always() }} | |||
| 32 | + | ST_CANCELLED: ${{ cancelled() }} | |||
| 33 | + | run: env | grep '^ST_' | sort | |||
| 34 | + | `, []string{ | |||
| 35 | + | "w.yml (Line: 10, Col: 23): Unrecognized function: 'success'. Located at position 1 within expression: success()", | |||
| 36 | + | "w.yml (Line: 11, Col: 23): Unrecognized function: 'failure'. Located at position 1 within expression: failure()", | |||
| 37 | + | "w.yml (Line: 12, Col: 22): Unrecognized function: 'always'. Located at position 1 within expression: always()", | |||
| 38 | + | "w.yml (Line: 13, Col: 25): Unrecognized function: 'cancelled'. Located at position 1 within expression: cancelled()", | |||
| 39 | + | }}, | |||
| 40 | + | {"expr-status-in-run", on + `jobs: | |||
| 41 | + | main: | |||
| 42 | + | runs-on: ubuntu-24.04 | |||
| 43 | + | steps: | |||
| 44 | + | - name: status functions in run | |||
| 45 | + | run: | | |||
| 46 | + | echo "success=${{ success() }}" | |||
| 47 | + | echo "failure=${{ failure() }}" | |||
| 48 | + | `, []string{ | |||
| 49 | + | "w.yml (Line: 9, Col: 14): Unrecognized function: 'success'. Located at position 1 within expression: success()", | |||
| 50 | + | "w.yml (Line: 9, Col: 14): Unrecognized function: 'failure'. Located at position 1 within expression: failure()", | |||
| 51 | + | }}, | |||
| 52 | + | {"invalid-needs unknown-need", on + `jobs: | |||
| 53 | + | main: | |||
| 54 | + | needs: nowhere | |||
| 55 | + | runs-on: ubuntu-24.04 | |||
| 56 | + | steps: | |||
| 57 | + | - run: echo SHOULD-NOT-RUN | |||
| 58 | + | `, []string{"w.yml (Line: 5, Col: 3): The workflow must contain at least one job with no dependencies."}}, | |||
| 59 | + | {"invalid-needs unknown-need-with-root", on + `jobs: | |||
| 60 | + | root: | |||
| 61 | + | runs-on: ubuntu-24.04 | |||
| 62 | + | steps: | |||
| 63 | + | - run: echo SHOULD-NOT-RUN | |||
| 64 | + | dependent: | |||
| 65 | + | needs: [root, nowhere] | |||
| 66 | + | runs-on: ubuntu-24.04 | |||
| 67 | + | steps: | |||
| 68 | + | - run: echo SHOULD-NOT-RUN | |||
| 69 | + | `, []string{"w.yml (Line: 10, Col: 19): Job 'dependent' depends on unknown job 'nowhere'."}}, | |||
| 70 | + | {"invalid-needs cycle", on + `jobs: | |||
| 71 | + | a: | |||
| 72 | + | needs: b | |||
| 73 | + | runs-on: ubuntu-24.04 | |||
| 74 | + | steps: | |||
| 75 | + | - run: echo SHOULD-NOT-RUN | |||
| 76 | + | b: | |||
| 77 | + | needs: a | |||
| 78 | + | runs-on: ubuntu-24.04 | |||
| 79 | + | steps: | |||
| 80 | + | - run: echo SHOULD-NOT-RUN | |||
| 81 | + | `, []string{"w.yml (Line: 5, Col: 3): The workflow must contain at least one job with no dependencies."}}, | |||
| 82 | + | {"invalid-needs cycle-with-root", on + `jobs: | |||
| 83 | + | root: | |||
| 84 | + | runs-on: ubuntu-24.04 | |||
| 85 | + | steps: | |||
| 86 | + | - run: echo SHOULD-NOT-RUN | |||
| 87 | + | a: | |||
| 88 | + | needs: [root, b] | |||
| 89 | + | runs-on: ubuntu-24.04 | |||
| 90 | + | steps: | |||
| 91 | + | - run: echo SHOULD-NOT-RUN | |||
| 92 | + | b: | |||
| 93 | + | needs: a | |||
| 94 | + | runs-on: ubuntu-24.04 | |||
| 95 | + | steps: | |||
| 96 | + | - run: echo SHOULD-NOT-RUN | |||
| 97 | + | `, []string{ | |||
| 98 | + | "w.yml (Line: 10, Col: 19): Job 'a' depends on job 'b' which creates a cycle in the dependency graph.", | |||
| 99 | + | "w.yml (Line: 15, Col: 12): Job 'b' depends on job 'a' which creates a cycle in the dependency graph.", | |||
| 100 | + | }}, | |||
| 101 | + | {"invalid-schema unknown-key", on + `jobs: | |||
| 102 | + | main: | |||
| 103 | + | runs-on: ubuntu-24.04 | |||
| 104 | + | unknown-key: true | |||
| 105 | + | steps: | |||
| 106 | + | - run: echo SHOULD-NOT-RUN | |||
| 107 | + | `, []string{"w.yml (Line: 7, Col: 5): Unexpected value 'unknown-key'"}}, | |||
| 108 | + | {"invalid-schema no-runs-on", on + `jobs: | |||
| 109 | + | main: | |||
| 110 | + | steps: | |||
| 111 | + | - run: echo SHOULD-NOT-RUN | |||
| 112 | + | `, []string{"w.yml (Line: 6, Col: 5): Required property is missing: runs-on"}}, | |||
| 113 | + | {"invalid-schema empty-step", on + `jobs: | |||
| 114 | + | main: | |||
| 115 | + | runs-on: ubuntu-24.04 | |||
| 116 | + | steps: | |||
| 117 | + | - name: nothing to run | |||
| 118 | + | `, []string{"w.yml (Line: 8, Col: 9): There's not enough info to determine what you meant. Add one of these properties: run, shell, uses, with, working-directory"}}, | |||
| 119 | + | } | |||
| 120 | + | for _, tc := range tests { | |||
| 121 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 122 | + | _, errs := Parse("w.yml", []byte(tc.src)) | |||
| 123 | + | var got []string | |||
| 124 | + | for _, e := range errs { | |||
| 125 | + | got = append(got, e.Error()) | |||
| 126 | + | } | |||
| 127 | + | if !reflect.DeepEqual(got, tc.want) { | |||
| 128 | + | t.Errorf("errors:\n got %q\n want %q", got, tc.want) | |||
| 129 | + | } | |||
| 130 | + | }) | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | // TestRecordedValid checks that the valid siblings of the invalid scenarios | |||
| 135 | + | // and the status functions in conditions, which GitHub accepted, parse. | |||
| 136 | + | func TestRecordedValid(t *testing.T) { | |||
| 137 | + | for name, src := range map[string]string{ | |||
| 138 | + | "valid sibling": "name: valid-sibling\non:\n push:\n branches: ['scn/**']\n tags: ['scn-*']\njobs:\n main:\n runs-on: ubuntu-24.04\n steps:\n - run: echo valid-sibling-ran\n", | |||
| 139 | + | "status functions in if": "on: push\njobs:\n main:\n runs-on: x\n if: success() || failure()\n steps:\n - if: always() && !cancelled()\n run: echo\n", | |||
| 140 | + | } { | |||
| 141 | + | if _, errs := Parse("w.yml", []byte(src)); len(errs) > 0 { | |||
| 142 | + | t.Errorf("%s: %v", name, errs) | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | } | |||
M workflow/skip.go +33 -35
| @@ -6,53 +6,51 @@ import "strings" | |||
| 6 | 6 | ||
| 7 | 7 | var skipMarkers = []string{"[skip ci]", "[ci skip]", "[no ci]", "[skip actions]", "[actions skip]"} | |
| 8 | 8 | ||
| 9 | - | // SkipCI reports whether the message of the head commit of a push tells | |
| 10 | - | // GitHub not to run workflows for it: it contains [skip ci], [ci skip], | |
| 11 | - | // [no ci], [skip actions] or [actions skip], or it ends with a | |
| 12 | - | // "skip-checks: true" (or "skip-checks:true") trailer in its last paragraph. | |
| 9 | + | // SkipCI reports whether the message of the head commit of a push (or of the | |
| 10 | + | // commit a tag points at) tells GitHub not to run workflows for it. As | |
| 11 | + | // recorded on github.com (scenarios skip-directives, skip-directives-placement, | |
| 12 | + | // skip-checks-trailer and skip-directive-tag): | |
| 13 | 13 | // | |
| 14 | - | // TODO(verify with recording): the following is assumed. Markers are matched | |
| 15 | - | // without regard to case, anywhere in the message, subject or body. GitHub | |
| 16 | - | // documents the trailer as "two empty lines followed by skip-checks: true"; | |
| 17 | - | // here one blank line is enough, the trailer must be in the last paragraph | |
| 18 | - | // and not the first one, its key and value are case-insensitive, and other | |
| 19 | - | // trailers may share its paragraph. Whether GitHub looks at the head commit | |
| 20 | - | // only, or at every commit of the push, is not settled by its documentation. | |
| 14 | + | // - the markers [skip ci], [ci skip], [no ci], [skip actions] and | |
| 15 | + | // [actions skip] skip anywhere in the subject or the body, and are | |
| 16 | + | // case-sensitive: [SKIP CI] and [Skip Ci] do not skip. Near misses do | |
| 17 | + | // not skip either: [skip-ci], "skip ci" without brackets, [skip ci please] | |
| 18 | + | // and the markers ***NO_CI*** and ***SKIP_CI***; | |
| 19 | + | // - the trailer "skip-checks: true" skips only as the end of the message, | |
| 20 | + | // preceded by two blank lines ("x\n\n\nskip-checks: true"). The space | |
| 21 | + | // after the colon is optional and trailing white space is ignored. It does | |
| 22 | + | // not skip with one blank line or none, with text after it, after another | |
| 23 | + | // trailer, with another value ("false", "yes"), or with any change of case | |
| 24 | + | // in the key or the value. | |
| 25 | + | // | |
| 26 | + | // Not recorded, so assumed: only the head commit of a push is looked at (the | |
| 27 | + | // scenarios push one commit at a time); more than two blank lines before the | |
| 28 | + | // trailer count like two; a line holding only spaces or tabs is not blank | |
| 29 | + | // (the two lines before the trailer must be empty); trailing blank lines and | |
| 30 | + | // CRLF line ends are ignored; more than one space after the colon is not | |
| 31 | + | // accepted; and a message made of the trailer alone does not skip. | |
| 21 | 32 | func SkipCI(headCommitMessage string) bool { | |
| 22 | - | lower := strings.ToLower(headCommitMessage) | |
| 23 | 33 | for _, m := range skipMarkers { | |
| 24 | - | if strings.Contains(lower, m) { | |
| 34 | + | if strings.Contains(headCommitMessage, m) { | |
| 25 | 35 | return true | |
| 26 | 36 | } | |
| 27 | 37 | } | |
| 28 | 38 | return hasSkipChecksTrailer(headCommitMessage) | |
| 29 | 39 | } | |
| 30 | 40 | ||
| 41 | + | // hasSkipChecksTrailer reports whether the message ends with a | |
| 42 | + | // "skip-checks: true" line preceded by two empty lines and some text. | |
| 31 | 43 | func hasSkipChecksTrailer(msg string) bool { | |
| 32 | 44 | msg = strings.ReplaceAll(msg, "\r\n", "\n") | |
| 33 | - | var paras [][]string | |
| 34 | - | var cur []string | |
| 35 | - | for _, line := range strings.Split(msg, "\n") { | |
| 36 | - | if strings.TrimSpace(line) == "" { | |
| 37 | - | if len(cur) > 0 { | |
| 38 | - | paras = append(paras, cur) | |
| 39 | - | cur = nil | |
| 40 | - | } | |
| 41 | - | continue | |
| 42 | - | } | |
| 43 | - | cur = append(cur, line) | |
| 44 | - | } | |
| 45 | - | if len(cur) > 0 { | |
| 46 | - | paras = append(paras, cur) | |
| 47 | - | } | |
| 48 | - | if len(paras) < 2 { | |
| 45 | + | msg = strings.TrimRight(msg, " \t\n") | |
| 46 | + | lines := strings.Split(msg, "\n") | |
| 47 | + | n := len(lines) | |
| 48 | + | if n < 4 { | |
| 49 | 49 | return false | |
| 50 | 50 | } | |
| 51 | - | for _, line := range paras[len(paras)-1] { | |
| 52 | - | key, val, ok := strings.Cut(line, ":") | |
| 53 | - | if ok && strings.EqualFold(strings.TrimSpace(key), "skip-checks") && strings.EqualFold(strings.TrimSpace(val), "true") { | |
| 54 | - | return true | |
| 55 | - | } | |
| 51 | + | val, ok := strings.CutPrefix(lines[n-1], "skip-checks:") | |
| 52 | + | if !ok || strings.TrimPrefix(val, " ") != "true" { | |
| 53 | + | return false | |
| 56 | 54 | } | |
| 57 | - | return false | |
| 55 | + | return lines[n-2] == "" && lines[n-3] == "" && strings.TrimSpace(strings.Join(lines[:n-3], "")) != "" | |
| 58 | 56 | } | |
| @@ -6,53 +6,51 @@ import "strings" | |||||
| 6 | 6 | ||||
| 7 | var skipMarkers = []string{"[skip ci]", "[ci skip]", "[no ci]", "[skip actions]", "[actions skip]"} | 7 | var skipMarkers = []string{"[skip ci]", "[ci skip]", "[no ci]", "[skip actions]", "[actions skip]"} | ||
| 8 | 8 | ||||
| 9 | - | // SkipCI reports whether the message of the head commit of a push tells | 9 | + | // SkipCI reports whether the message of the head commit of a push (or of the |
| 10 | - | // GitHub not to run workflows for it: it contains [skip ci], [ci skip], | 10 | + | // commit a tag points at) tells GitHub not to run workflows for it. As |
| 11 | - | // [no ci], [skip actions] or [actions skip], or it ends with a | 11 | + | // recorded on github.com (scenarios skip-directives, skip-directives-placement, |
| 12 | - | // "skip-checks: true" (or "skip-checks:true") trailer in its last paragraph. | 12 | + | // skip-checks-trailer and skip-directive-tag): |
| 13 | // | 13 | // | ||
| 14 | - | // TODO(verify with recording): the following is assumed. Markers are matched | 14 | + | // - the markers [skip ci], [ci skip], [no ci], [skip actions] and |
| 15 | - | // without regard to case, anywhere in the message, subject or body. GitHub | 15 | + | // [actions skip] skip anywhere in the subject or the body, and are |
| 16 | - | // documents the trailer as "two empty lines followed by skip-checks: true"; | 16 | + | // case-sensitive: [SKIP CI] and [Skip Ci] do not skip. Near misses do |
| 17 | - | // here one blank line is enough, the trailer must be in the last paragraph | 17 | + | // not skip either: [skip-ci], "skip ci" without brackets, [skip ci please] |
| 18 | - | // and not the first one, its key and value are case-insensitive, and other | 18 | + | // and the markers ***NO_CI*** and ***SKIP_CI***; |
| 19 | - | // trailers may share its paragraph. Whether GitHub looks at the head commit | 19 | + | // - the trailer "skip-checks: true" skips only as the end of the message, |
| 20 | - | // only, or at every commit of the push, is not settled by its documentation. | 20 | + | // preceded by two blank lines ("x\n\n\nskip-checks: true"). The space |
| 21 | + | // after the colon is optional and trailing white space is ignored. It does | |||
| 22 | + | // not skip with one blank line or none, with text after it, after another | |||
| 23 | + | // trailer, with another value ("false", "yes"), or with any change of case | |||
| 24 | + | // in the key or the value. | |||
| 25 | + | // | |||
| 26 | + | // Not recorded, so assumed: only the head commit of a push is looked at (the | |||
| 27 | + | // scenarios push one commit at a time); more than two blank lines before the | |||
| 28 | + | // trailer count like two; a line holding only spaces or tabs is not blank | |||
| 29 | + | // (the two lines before the trailer must be empty); trailing blank lines and | |||
| 30 | + | // CRLF line ends are ignored; more than one space after the colon is not | |||
| 31 | + | // accepted; and a message made of the trailer alone does not skip. | |||
| 21 | func SkipCI(headCommitMessage string) bool { | 32 | func SkipCI(headCommitMessage string) bool { | ||
| 22 | - | lower := strings.ToLower(headCommitMessage) | |||
| 23 | for _, m := range skipMarkers { | 33 | for _, m := range skipMarkers { | ||
| 24 | - | if strings.Contains(lower, m) { | 34 | + | if strings.Contains(headCommitMessage, m) { |
| 25 | return true | 35 | return true | ||
| 26 | } | 36 | } | ||
| 27 | } | 37 | } | ||
| 28 | return hasSkipChecksTrailer(headCommitMessage) | 38 | return hasSkipChecksTrailer(headCommitMessage) | ||
| 29 | } | 39 | } | ||
| 30 | 40 | ||||
| 41 | + | // hasSkipChecksTrailer reports whether the message ends with a | |||
| 42 | + | // "skip-checks: true" line preceded by two empty lines and some text. | |||
| 31 | func hasSkipChecksTrailer(msg string) bool { | 43 | func hasSkipChecksTrailer(msg string) bool { | ||
| 32 | msg = strings.ReplaceAll(msg, "\r\n", "\n") | 44 | msg = strings.ReplaceAll(msg, "\r\n", "\n") | ||
| 33 | - | var paras [][]string | 45 | + | msg = strings.TrimRight(msg, " \t\n") |
| 34 | - | var cur []string | 46 | + | lines := strings.Split(msg, "\n") |
| 35 | - | for _, line := range strings.Split(msg, "\n") { | 47 | + | n := len(lines) |
| 36 | - | if strings.TrimSpace(line) == "" { | 48 | + | if n < 4 { |
| 37 | - | if len(cur) > 0 { | |||
| 38 | - | paras = append(paras, cur) | |||
| 39 | - | cur = nil | |||
| 40 | - | } | |||
| 41 | - | continue | |||
| 42 | - | } | |||
| 43 | - | cur = append(cur, line) | |||
| 44 | - | } | |||
| 45 | - | if len(cur) > 0 { | |||
| 46 | - | paras = append(paras, cur) | |||
| 47 | - | } | |||
| 48 | - | if len(paras) < 2 { | |||
| 49 | return false | 49 | return false | ||
| 50 | } | 50 | } | ||
| 51 | - | for _, line := range paras[len(paras)-1] { | 51 | + | val, ok := strings.CutPrefix(lines[n-1], "skip-checks:") |
| 52 | - | key, val, ok := strings.Cut(line, ":") | 52 | + | if !ok || strings.TrimPrefix(val, " ") != "true" { |
| 53 | - | if ok && strings.EqualFold(strings.TrimSpace(key), "skip-checks") && strings.EqualFold(strings.TrimSpace(val), "true") { | 53 | + | return false |
| 54 | - | return true | |||
| 55 | - | } | |||
| 56 | } | 54 | } | ||
| 57 | - | return false | 55 | + | return lines[n-2] == "" && lines[n-3] == "" && strings.TrimSpace(strings.Join(lines[:n-3], "")) != "" |
| 58 | } | 56 | } | ||
M workflow/skip_test.go +59 -32
| @@ -4,44 +4,71 @@ package workflow | |||
| 4 | 4 | ||
| 5 | 5 | import "testing" | |
| 6 | 6 | ||
| 7 | + | // TestSkipCI replays the commit messages pushed by the recorded scenarios | |
| 8 | + | // skip-directives, skip-directives-placement, skip-checks-trailer and | |
| 9 | + | // skip-directive-tag. skip is true when GitHub started no run. | |
| 7 | 10 | func TestSkipCI(t *testing.T) { | |
| 8 | 11 | tests := []struct { | |
| 9 | - | name, msg string | |
| 10 | - | want bool | |
| 12 | + | scenario, msg string | |
| 13 | + | skip bool | |
| 11 | 14 | }{ | |
| 12 | - | {"empty", "", false}, | |
| 13 | - | {"plain", "fix: a bug", false}, | |
| 14 | - | {"skip ci", "docs: typo [skip ci]", true}, | |
| 15 | - | {"ci skip", "docs: typo [ci skip]", true}, | |
| 16 | - | {"no ci", "docs: typo [no ci]", true}, | |
| 17 | - | {"skip actions", "docs: typo [skip actions]", true}, | |
| 18 | - | {"actions skip", "docs: typo [actions skip]", true}, | |
| 19 | - | {"in body", "docs: typo\n\nlong text [skip ci] more", true}, | |
| 20 | - | {"in subject start", "[skip ci] docs", true}, | |
| 21 | - | {"no brackets", "skip ci", false}, | |
| 22 | - | {"other brackets", "[skip tests]", false}, | |
| 23 | - | {"partial", "[skip ci]", false}, | |
| 24 | - | // TODO(verify with recording): case of the markers. | |
| 25 | - | {"upper case marker", "docs [SKIP CI]", true}, | |
| 26 | - | {"mixed case marker", "docs [Skip Ci]", true}, | |
| 15 | + | // skip-directives | |
| 16 | + | {"skip-directives 1", "change one [skip ci]", true}, | |
| 17 | + | {"skip-directives 2", "change two [ci skip]", true}, | |
| 18 | + | {"skip-directives 3", "change three [no ci]", true}, | |
| 19 | + | {"skip-directives 4", "change four [skip actions]", true}, | |
| 20 | + | {"skip-directives 5", "change five [actions skip]", true}, | |
| 21 | + | {"skip-directives 6", "change six, no directive", false}, | |
| 22 | + | ||
| 23 | + | // skip-directives-placement | |
| 24 | + | {"placement 1 body", "subject\n\nbody line [skip ci] more body", true}, | |
| 25 | + | {"placement 2 mid-subject", "prefix [skip ci] suffix", true}, | |
| 26 | + | {"placement 3 upper case", "upper case [SKIP CI]", false}, | |
| 27 | + | {"placement 4 mixed case", "mixed case [Skip Ci]", false}, | |
| 28 | + | {"placement 5 near miss dash", "near miss [skip-ci]", false}, | |
| 29 | + | {"placement 6 no brackets", "near miss skip ci without brackets", false}, | |
| 30 | + | {"placement 7 extra word", "near miss [skip ci please]", false}, | |
| 31 | + | {"placement 8 NO_CI", "marker ***NO_CI*** in subject", false}, | |
| 32 | + | {"placement 9 SKIP_CI", "marker ***SKIP_CI*** in subject", false}, | |
| 33 | + | ||
| 34 | + | // skip-checks-trailer | |
| 35 | + | {"trailer 1 one blank line", "trailer true\n\nskip-checks: true", false}, | |
| 36 | + | {"trailer 2 false, one blank line", "trailer false\n\nskip-checks: false", false}, | |
| 37 | + | {"trailer 3 upper, one blank line", "trailer upper\n\nSkip-Checks: true", false}, | |
| 38 | + | {"trailer 4 other trailer first", "trailer after other trailer\n\nSigned-off-by: A Person <a@example.invalid>\nskip-checks: true", false}, | |
| 39 | + | {"trailer 5 in the subject", "not a trailer, in the subject skip-checks: true", false}, | |
| 40 | + | {"trailer 6 no blank line", "trailer without blank line\nskip-checks: true", false}, | |
| 41 | + | {"trailer 7 two blank lines", "two blank lines true\n\n\nskip-checks: true", true}, | |
| 42 | + | {"trailer 8 no space", "two blank lines no space\n\n\nskip-checks:true", true}, | |
| 43 | + | {"trailer 9 false", "two blank lines false\n\n\nskip-checks: false", false}, | |
| 44 | + | {"trailer 10 value upper", "two blank lines upper\n\n\nSkip-Checks: TRUE", false}, | |
| 45 | + | {"trailer 11 text after", "two blank lines then more text\n\n\nskip-checks: true\nand more", false}, | |
| 46 | + | {"trailer 12 after other trailer", "two blank lines after other trailer\n\n\nSigned-off-by: A Person <a@example.invalid>\nskip-checks: true", false}, | |
| 47 | + | {"trailer 13 trailing spaces", "two blank lines trailing spaces\n\n\nskip-checks: true ", true}, | |
| 48 | + | {"trailer 14 yes", "two blank lines yes\n\n\nskip-checks: yes", false}, | |
| 49 | + | {"trailer 15 key upper", "two blank lines key upper value lower\n\n\nSkip-Checks: true", false}, | |
| 50 | + | {"trailer 16 value upper", "two blank lines key lower value upper\n\n\nskip-checks: TRUE", false}, | |
| 51 | + | {"trailer 17 key mixed", "two blank lines key mixed\n\n\nSkip-checks: true", false}, | |
| 52 | + | {"trailer 18 plain", "plain commit", false}, | |
| 27 | 53 | ||
| 28 | - | // skip-checks trailer. | |
| 29 | - | {"trailer", "feat: x\n\nskip-checks: true", true}, | |
| 30 | - | {"trailer no space", "feat: x\n\nskip-checks:true", true}, | |
| 31 | - | {"trailer with body", "feat: x\n\nbody text\n\nskip-checks: true", true}, | |
| 32 | - | {"trailer after two blank lines", "feat: x\n\n\nskip-checks: true\n", true}, | |
| 33 | - | {"trailer crlf", "feat: x\r\n\r\nskip-checks: true\r\n", true}, | |
| 34 | - | {"trailer with other trailers", "feat: x\n\nSigned-off-by: A <a@x>\nskip-checks: true", true}, | |
| 35 | - | {"trailer false", "feat: x\n\nskip-checks: false", false}, | |
| 36 | - | {"trailer not last paragraph", "feat: x\n\nskip-checks: true\n\nmore text", false}, | |
| 37 | - | {"trailer in first paragraph only", "skip-checks: true", false}, | |
| 38 | - | {"trailer in subject", "feat: x skip-checks: true", false}, | |
| 39 | - | // TODO(verify with recording): case of key and value. | |
| 40 | - | {"trailer upper", "feat: x\n\nSkip-Checks: TRUE", true}, | |
| 54 | + | // skip-directive-tag: the message of the commit a tag points at. | |
| 55 | + | {"tag 1", "tagged [skip ci]", true}, | |
| 56 | + | {"tag 2", "tagged normally", false}, | |
| 57 | + | ||
| 58 | + | // Not recorded: see the doc comment of SkipCI. | |
| 59 | + | {"trailing newline", "x\n\n\nskip-checks: true\n", true}, | |
| 60 | + | {"crlf", "x\r\n\r\n\r\nskip-checks: true\r\n", true}, | |
| 61 | + | {"three blank lines", "x\n\n\n\nskip-checks: true", true}, | |
| 62 | + | {"trailer alone", "\n\n\nskip-checks: true", false}, | |
| 63 | + | // A line with only white space is not blank (GO-006, assumed). | |
| 64 | + | {"space-only lines before trailer", "x\n \n \nskip-checks: true", false}, | |
| 65 | + | {"one space-only line before trailer", "x\n\n \nskip-checks: true", false}, | |
| 66 | + | {"tab-only lines before trailer", "x\n\t\n\t\nskip-checks: true", false}, | |
| 67 | + | {"empty", "", false}, | |
| 41 | 68 | } | |
| 42 | 69 | for _, tc := range tests { | |
| 43 | - | if got := SkipCI(tc.msg); got != tc.want { | |
| 44 | - | t.Errorf("%s: SkipCI(%q) = %v, want %v", tc.name, tc.msg, got, tc.want) | |
| 70 | + | if got := SkipCI(tc.msg); got != tc.skip { | |
| 71 | + | t.Errorf("%s: SkipCI(%q) = %v, want %v", tc.scenario, tc.msg, got, tc.skip) | |
| 45 | 72 | } | |
| 46 | 73 | } | |
| 47 | 74 | } | |
| @@ -4,44 +4,71 @@ package workflow | |||||
| 4 | 4 | ||||
| 5 | import "testing" | 5 | import "testing" | ||
| 6 | 6 | ||||
| 7 | + | // TestSkipCI replays the commit messages pushed by the recorded scenarios | |||
| 8 | + | // skip-directives, skip-directives-placement, skip-checks-trailer and | |||
| 9 | + | // skip-directive-tag. skip is true when GitHub started no run. | |||
| 7 | func TestSkipCI(t *testing.T) { | 10 | func TestSkipCI(t *testing.T) { | ||
| 8 | tests := []struct { | 11 | tests := []struct { | ||
| 9 | - | name, msg string | 12 | + | scenario, msg string |
| 10 | - | want bool | 13 | + | skip bool |
| 11 | }{ | 14 | }{ | ||
| 12 | - | {"empty", "", false}, | 15 | + | // skip-directives |
| 13 | - | {"plain", "fix: a bug", false}, | 16 | + | {"skip-directives 1", "change one [skip ci]", true}, |
| 14 | - | {"skip ci", "docs: typo [skip ci]", true}, | 17 | + | {"skip-directives 2", "change two [ci skip]", true}, |
| 15 | - | {"ci skip", "docs: typo [ci skip]", true}, | 18 | + | {"skip-directives 3", "change three [no ci]", true}, |
| 16 | - | {"no ci", "docs: typo [no ci]", true}, | 19 | + | {"skip-directives 4", "change four [skip actions]", true}, |
| 17 | - | {"skip actions", "docs: typo [skip actions]", true}, | 20 | + | {"skip-directives 5", "change five [actions skip]", true}, |
| 18 | - | {"actions skip", "docs: typo [actions skip]", true}, | 21 | + | {"skip-directives 6", "change six, no directive", false}, |
| 19 | - | {"in body", "docs: typo\n\nlong text [skip ci] more", true}, | 22 | + | |
| 20 | - | {"in subject start", "[skip ci] docs", true}, | 23 | + | // skip-directives-placement |
| 21 | - | {"no brackets", "skip ci", false}, | 24 | + | {"placement 1 body", "subject\n\nbody line [skip ci] more body", true}, |
| 22 | - | {"other brackets", "[skip tests]", false}, | 25 | + | {"placement 2 mid-subject", "prefix [skip ci] suffix", true}, |
| 23 | - | {"partial", "[skip ci]", false}, | 26 | + | {"placement 3 upper case", "upper case [SKIP CI]", false}, |
| 24 | - | // TODO(verify with recording): case of the markers. | 27 | + | {"placement 4 mixed case", "mixed case [Skip Ci]", false}, |
| 25 | - | {"upper case marker", "docs [SKIP CI]", true}, | 28 | + | {"placement 5 near miss dash", "near miss [skip-ci]", false}, |
| 26 | - | {"mixed case marker", "docs [Skip Ci]", true}, | 29 | + | {"placement 6 no brackets", "near miss skip ci without brackets", false}, |
| 30 | + | {"placement 7 extra word", "near miss [skip ci please]", false}, | |||
| 31 | + | {"placement 8 NO_CI", "marker ***NO_CI*** in subject", false}, | |||
| 32 | + | {"placement 9 SKIP_CI", "marker ***SKIP_CI*** in subject", false}, | |||
| 33 | + | ||||
| 34 | + | // skip-checks-trailer | |||
| 35 | + | {"trailer 1 one blank line", "trailer true\n\nskip-checks: true", false}, | |||
| 36 | + | {"trailer 2 false, one blank line", "trailer false\n\nskip-checks: false", false}, | |||
| 37 | + | {"trailer 3 upper, one blank line", "trailer upper\n\nSkip-Checks: true", false}, | |||
| 38 | + | {"trailer 4 other trailer first", "trailer after other trailer\n\nSigned-off-by: A Person <a@example.invalid>\nskip-checks: true", false}, | |||
| 39 | + | {"trailer 5 in the subject", "not a trailer, in the subject skip-checks: true", false}, | |||
| 40 | + | {"trailer 6 no blank line", "trailer without blank line\nskip-checks: true", false}, | |||
| 41 | + | {"trailer 7 two blank lines", "two blank lines true\n\n\nskip-checks: true", true}, | |||
| 42 | + | {"trailer 8 no space", "two blank lines no space\n\n\nskip-checks:true", true}, | |||
| 43 | + | {"trailer 9 false", "two blank lines false\n\n\nskip-checks: false", false}, | |||
| 44 | + | {"trailer 10 value upper", "two blank lines upper\n\n\nSkip-Checks: TRUE", false}, | |||
| 45 | + | {"trailer 11 text after", "two blank lines then more text\n\n\nskip-checks: true\nand more", false}, | |||
| 46 | + | {"trailer 12 after other trailer", "two blank lines after other trailer\n\n\nSigned-off-by: A Person <a@example.invalid>\nskip-checks: true", false}, | |||
| 47 | + | {"trailer 13 trailing spaces", "two blank lines trailing spaces\n\n\nskip-checks: true ", true}, | |||
| 48 | + | {"trailer 14 yes", "two blank lines yes\n\n\nskip-checks: yes", false}, | |||
| 49 | + | {"trailer 15 key upper", "two blank lines key upper value lower\n\n\nSkip-Checks: true", false}, | |||
| 50 | + | {"trailer 16 value upper", "two blank lines key lower value upper\n\n\nskip-checks: TRUE", false}, | |||
| 51 | + | {"trailer 17 key mixed", "two blank lines key mixed\n\n\nSkip-checks: true", false}, | |||
| 52 | + | {"trailer 18 plain", "plain commit", false}, | |||
| 27 | 53 | ||||
| 28 | - | // skip-checks trailer. | 54 | + | // skip-directive-tag: the message of the commit a tag points at. |
| 29 | - | {"trailer", "feat: x\n\nskip-checks: true", true}, | 55 | + | {"tag 1", "tagged [skip ci]", true}, |
| 30 | - | {"trailer no space", "feat: x\n\nskip-checks:true", true}, | 56 | + | {"tag 2", "tagged normally", false}, |
| 31 | - | {"trailer with body", "feat: x\n\nbody text\n\nskip-checks: true", true}, | 57 | + | |
| 32 | - | {"trailer after two blank lines", "feat: x\n\n\nskip-checks: true\n", true}, | 58 | + | // Not recorded: see the doc comment of SkipCI. |
| 33 | - | {"trailer crlf", "feat: x\r\n\r\nskip-checks: true\r\n", true}, | 59 | + | {"trailing newline", "x\n\n\nskip-checks: true\n", true}, |
| 34 | - | {"trailer with other trailers", "feat: x\n\nSigned-off-by: A <a@x>\nskip-checks: true", true}, | 60 | + | {"crlf", "x\r\n\r\n\r\nskip-checks: true\r\n", true}, |
| 35 | - | {"trailer false", "feat: x\n\nskip-checks: false", false}, | 61 | + | {"three blank lines", "x\n\n\n\nskip-checks: true", true}, |
| 36 | - | {"trailer not last paragraph", "feat: x\n\nskip-checks: true\n\nmore text", false}, | 62 | + | {"trailer alone", "\n\n\nskip-checks: true", false}, |
| 37 | - | {"trailer in first paragraph only", "skip-checks: true", false}, | 63 | + | // A line with only white space is not blank (GO-006, assumed). |
| 38 | - | {"trailer in subject", "feat: x skip-checks: true", false}, | 64 | + | {"space-only lines before trailer", "x\n \n \nskip-checks: true", false}, |
| 39 | - | // TODO(verify with recording): case of key and value. | 65 | + | {"one space-only line before trailer", "x\n\n \nskip-checks: true", false}, |
| 40 | - | {"trailer upper", "feat: x\n\nSkip-Checks: TRUE", true}, | 66 | + | {"tab-only lines before trailer", "x\n\t\n\t\nskip-checks: true", false}, |
| 67 | + | {"empty", "", false}, | |||
| 41 | } | 68 | } | ||
| 42 | for _, tc := range tests { | 69 | for _, tc := range tests { | ||
| 43 | - | if got := SkipCI(tc.msg); got != tc.want { | 70 | + | if got := SkipCI(tc.msg); got != tc.skip { |
| 44 | - | t.Errorf("%s: SkipCI(%q) = %v, want %v", tc.name, tc.msg, got, tc.want) | 71 | + | t.Errorf("%s: SkipCI(%q) = %v, want %v", tc.scenario, tc.msg, got, tc.skip) |
| 45 | } | 72 | } | ||
| 46 | } | 73 | } | ||
| 47 | } | 74 | } | ||
M workflow/strategy.go +11 -5
| @@ -134,7 +134,12 @@ func (j *Job) EvaluateStrategy(ctx context.Context, env template.Env) (*Strategy | |||
| 134 | 134 | if err != nil { | |
| 135 | 135 | return nil, err | |
| 136 | 136 | } | |
| 137 | - | return ParseStrategy(ctx, v) | |
| 137 | + | st, err := ParseStrategy(ctx, v) | |
| 138 | + | var limit *configurationLimitError | |
| 139 | + | if errors.As(err, &limit) { | |
| 140 | + | return nil, &configurationLimitError{Job: j.ID, Count: limit.Count} | |
| 141 | + | } | |
| 142 | + | return st, err | |
| 138 | 143 | } | |
| 139 | 144 | ||
| 140 | 145 | // MatrixEnv returns env with the "matrix" and "strategy" contexts of | |
| @@ -158,11 +163,12 @@ func (s *Strategy) MatrixEnv(env template.Env, index int) (template.Env, error) | |||
| 158 | 163 | // DisplayName evaluates the job's name for combination index of its | |
| 159 | 164 | // strategy: the name token (or the job id) with the matrix values added in | |
| 160 | 165 | // parentheses, unless the name is set and reads the matrix context, in which | |
| 161 | - | // case it is used as is. env must already include the matrix and strategy | |
| 162 | - | // contexts, see [Strategy.MatrixEnv]. | |
| 166 | + | // case it is used as is. Null and empty values are not listed. env must | |
| 167 | + | // already include the matrix and strategy contexts, see [Strategy.MatrixEnv]. | |
| 163 | 168 | // | |
| 164 | - | // TODO(verify with recording): GitHub adds the values to the name unless the | |
| 165 | - | // name is set and refers to the matrix. | |
| 169 | + | // Recorded on github.com (matrix-job-name): "default-name (a, 1)", a custom | |
| 170 | + | // name using the matrix as is, and a custom name that does not read the | |
| 171 | + | // matrix followed by the values ("same name for every leg (1)"). | |
| 166 | 172 | func (j *Job) DisplayName(ctx context.Context, env template.Env, s *Strategy, index int) (string, error) { | |
| 167 | 173 | if index < 0 || index >= len(s.Combos) { | |
| 168 | 174 | return "", fmt.Errorf("matrix combination %d out of range (%d combinations)", index, len(s.Combos)) | |
| @@ -134,7 +134,12 @@ func (j *Job) EvaluateStrategy(ctx context.Context, env template.Env) (*Strategy | |||||
| 134 | if err != nil { | 134 | if err != nil { | ||
| 135 | return nil, err | 135 | return nil, err | ||
| 136 | } | 136 | } | ||
| 137 | - | return ParseStrategy(ctx, v) | 137 | + | st, err := ParseStrategy(ctx, v) |
| 138 | + | var limit *configurationLimitError | |||
| 139 | + | if errors.As(err, &limit) { | |||
| 140 | + | return nil, &configurationLimitError{Job: j.ID, Count: limit.Count} | |||
| 141 | + | } | |||
| 142 | + | return st, err | |||
| 138 | } | 143 | } | ||
| 139 | 144 | ||||
| 140 | // MatrixEnv returns env with the "matrix" and "strategy" contexts of | 145 | // MatrixEnv returns env with the "matrix" and "strategy" contexts of | ||
| @@ -158,11 +163,12 @@ func (s *Strategy) MatrixEnv(env template.Env, index int) (template.Env, error) | |||||
| 158 | // DisplayName evaluates the job's name for combination index of its | 163 | // DisplayName evaluates the job's name for combination index of its | ||
| 159 | // strategy: the name token (or the job id) with the matrix values added in | 164 | // strategy: the name token (or the job id) with the matrix values added in | ||
| 160 | // parentheses, unless the name is set and reads the matrix context, in which | 165 | // parentheses, unless the name is set and reads the matrix context, in which | ||
| 161 | - | // case it is used as is. env must already include the matrix and strategy | 166 | + | // case it is used as is. Null and empty values are not listed. env must |
| 162 | - | // contexts, see [Strategy.MatrixEnv]. | 167 | + | // already include the matrix and strategy contexts, see [Strategy.MatrixEnv]. |
| 163 | // | 168 | // | ||
| 164 | - | // TODO(verify with recording): GitHub adds the values to the name unless the | 169 | + | // Recorded on github.com (matrix-job-name): "default-name (a, 1)", a custom |
| 165 | - | // name is set and refers to the matrix. | 170 | + | // name using the matrix as is, and a custom name that does not read the |
| 171 | + | // matrix followed by the values ("same name for every leg (1)"). | |||
| 166 | func (j *Job) DisplayName(ctx context.Context, env template.Env, s *Strategy, index int) (string, error) { | 172 | func (j *Job) DisplayName(ctx context.Context, env template.Env, s *Strategy, index int) (string, error) { | ||
| 167 | if index < 0 || index >= len(s.Combos) { | 173 | if index < 0 || index >= len(s.Combos) { | ||
| 168 | return "", fmt.Errorf("matrix combination %d out of range (%d combinations)", index, len(s.Combos)) | 174 | return "", fmt.Errorf("matrix combination %d out of range (%d combinations)", index, len(s.Combos)) | ||