Merge pull request 'fix(workflow): ErrExcludeUnknownKey and Workflow.Ancestors for GitHub parity' (#6) from fix/workflow-github-parity into main
4 files · +210 -5
M workflow/job_eval.go +39 -0
| @@ -224,3 +224,42 @@ func (w *Workflow) JobLevels() [][]*Job { | |||
| 224 | 224 | } | |
| 225 | 225 | return levels | |
| 226 | 226 | } | |
| 227 | + | ||
| 228 | + | // Ancestors returns the ids of every job the job with the given id depends on, | |
| 229 | + | // directly or through other jobs, in the order of the file. It is empty for a | |
| 230 | + | // job without needs and nil for an unknown id. | |
| 231 | + | // | |
| 232 | + | // GitHub's default condition of a job, success(), looks at this whole chain | |
| 233 | + | // and not only at the direct needs: a job that needs a job which ran through | |
| 234 | + | // always() after a failure is skipped, although its only direct need | |
| 235 | + | // succeeded. Recorded (needs-failed-dependency): chain-after-always needs | |
| 236 | + | // with-always, which needs broken; broken failed, with-always (if: always()) | |
| 237 | + | // succeeded, and chain-after-always was skipped. Likewise failure() is true | |
| 238 | + | // when any ancestor failed, as documented. | |
| 239 | + | func (w *Workflow) Ancestors(id string) []string { | |
| 240 | + | start := w.Job(id) | |
| 241 | + | if start == nil { | |
| 242 | + | return nil | |
| 243 | + | } | |
| 244 | + | seen := map[string]bool{id: true} | |
| 245 | + | stack := slices.Clone(start.Needs) | |
| 246 | + | for len(stack) > 0 { | |
| 247 | + | n := stack[len(stack)-1] | |
| 248 | + | stack = stack[:len(stack)-1] | |
| 249 | + | if seen[n] { | |
| 250 | + | continue | |
| 251 | + | } | |
| 252 | + | seen[n] = true | |
| 253 | + | if j := w.Job(n); j != nil { | |
| 254 | + | stack = append(stack, j.Needs...) | |
| 255 | + | } | |
| 256 | + | } | |
| 257 | + | delete(seen, id) | |
| 258 | + | out := []string{} | |
| 259 | + | for _, j := range w.Jobs { | |
| 260 | + | if seen[j.ID] { | |
| 261 | + | out = append(out, j.ID) | |
| 262 | + | } | |
| 263 | + | } | |
| 264 | + | return out | |
| 265 | + | } | |
| @@ -224,3 +224,42 @@ func (w *Workflow) JobLevels() [][]*Job { | |||||
| 224 | } | 224 | } | ||
| 225 | return levels | 225 | return levels | ||
| 226 | } | 226 | } | ||
| 227 | + | ||||
| 228 | + | // Ancestors returns the ids of every job the job with the given id depends on, | |||
| 229 | + | // directly or through other jobs, in the order of the file. It is empty for a | |||
| 230 | + | // job without needs and nil for an unknown id. | |||
| 231 | + | // | |||
| 232 | + | // GitHub's default condition of a job, success(), looks at this whole chain | |||
| 233 | + | // and not only at the direct needs: a job that needs a job which ran through | |||
| 234 | + | // always() after a failure is skipped, although its only direct need | |||
| 235 | + | // succeeded. Recorded (needs-failed-dependency): chain-after-always needs | |||
| 236 | + | // with-always, which needs broken; broken failed, with-always (if: always()) | |||
| 237 | + | // succeeded, and chain-after-always was skipped. Likewise failure() is true | |||
| 238 | + | // when any ancestor failed, as documented. | |||
| 239 | + | func (w *Workflow) Ancestors(id string) []string { | |||
| 240 | + | start := w.Job(id) | |||
| 241 | + | if start == nil { | |||
| 242 | + | return nil | |||
| 243 | + | } | |||
| 244 | + | seen := map[string]bool{id: true} | |||
| 245 | + | stack := slices.Clone(start.Needs) | |||
| 246 | + | for len(stack) > 0 { | |||
| 247 | + | n := stack[len(stack)-1] | |||
| 248 | + | stack = stack[:len(stack)-1] | |||
| 249 | + | if seen[n] { | |||
| 250 | + | continue | |||
| 251 | + | } | |||
| 252 | + | seen[n] = true | |||
| 253 | + | if j := w.Job(n); j != nil { | |||
| 254 | + | stack = append(stack, j.Needs...) | |||
| 255 | + | } | |||
| 256 | + | } | |||
| 257 | + | delete(seen, id) | |||
| 258 | + | out := []string{} | |||
| 259 | + | for _, j := range w.Jobs { | |||
| 260 | + | if seen[j.ID] { | |||
| 261 | + | out = append(out, j.ID) | |||
| 262 | + | } | |||
| 263 | + | } | |||
| 264 | + | return out | |||
| 265 | + | } | |||
M workflow/job_eval_test.go +68 -0
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | package workflow | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "reflect" | |
| 6 | 7 | "strings" | |
| 7 | 8 | "testing" | |
| 8 | 9 | ) | |
| @@ -200,3 +201,70 @@ jobs: | |||
| 200 | 201 | } | |
| 201 | 202 | } | |
| 202 | 203 | } | |
| 204 | + | ||
| 205 | + | // needsFailedDependency is a cut-down workflow of the recorded scenario of | |
| 206 | + | // that name (without with-failure, with-success and broken's outputs), plus a | |
| 207 | + | // made-up diamond job that GitHub did not run. | |
| 208 | + | const needsFailedDependency = `on: | |
| 209 | + | push: | |
| 210 | + | branches: ['scn/**'] | |
| 211 | + | jobs: | |
| 212 | + | broken: | |
| 213 | + | runs-on: ubuntu-24.04 | |
| 214 | + | steps: | |
| 215 | + | - run: exit 1 | |
| 216 | + | plain: | |
| 217 | + | needs: broken | |
| 218 | + | runs-on: ubuntu-24.04 | |
| 219 | + | steps: | |
| 220 | + | - run: echo SHOULD-NOT-RUN | |
| 221 | + | with-always: | |
| 222 | + | needs: broken | |
| 223 | + | if: always() | |
| 224 | + | runs-on: ubuntu-24.04 | |
| 225 | + | steps: | |
| 226 | + | - run: echo | |
| 227 | + | chain-after-always: | |
| 228 | + | needs: with-always | |
| 229 | + | runs-on: ubuntu-24.04 | |
| 230 | + | steps: | |
| 231 | + | - run: echo chain-after-always-runs-because-with-always-succeeded | |
| 232 | + | diamond: | |
| 233 | + | needs: [chain-after-always, plain, with-always] | |
| 234 | + | runs-on: ubuntu-24.04 | |
| 235 | + | steps: | |
| 236 | + | - run: echo | |
| 237 | + | ` | |
| 238 | + | ||
| 239 | + | // TestAncestors checks the transitive needs that GitHub's default success() | |
| 240 | + | // looks at (recorded: needs-failed-dependency, chain-after-always was skipped). | |
| 241 | + | func TestAncestors(t *testing.T) { | |
| 242 | + | w, errs := Parse("w.yml", []byte(needsFailedDependency)) | |
| 243 | + | if len(errs) > 0 { | |
| 244 | + | t.Fatal(errs) | |
| 245 | + | } | |
| 246 | + | for id, want := range map[string][]string{ | |
| 247 | + | "broken": {}, | |
| 248 | + | "plain": {"broken"}, | |
| 249 | + | "with-always": {"broken"}, | |
| 250 | + | "chain-after-always": {"broken", "with-always"}, | |
| 251 | + | "diamond": {"broken", "plain", "with-always", "chain-after-always"}, | |
| 252 | + | "nowhere": nil, | |
| 253 | + | } { | |
| 254 | + | if got := w.Ancestors(id); !reflect.DeepEqual(got, want) { | |
| 255 | + | t.Errorf("Ancestors(%q) = %q, want %q", id, got, want) | |
| 256 | + | } | |
| 257 | + | } | |
| 258 | + | } | |
| 259 | + | ||
| 260 | + | // TestAncestorsCycleAndUnknownNeed covers what Parse rejects but a hand-built | |
| 261 | + | // Workflow may hold: a cycle ends, the start job and unknown ids are left out. | |
| 262 | + | func TestAncestorsCycleAndUnknownNeed(t *testing.T) { | |
| 263 | + | w := &Workflow{Jobs: []*Job{ | |
| 264 | + | {ID: "a", Needs: []string{"b", "ghost"}}, | |
| 265 | + | {ID: "b", Needs: []string{"a"}}, | |
| 266 | + | }} | |
| 267 | + | if got := w.Ancestors("a"); !reflect.DeepEqual(got, []string{"b"}) { | |
| 268 | + | t.Errorf("Ancestors(a) = %q, want [b]", got) | |
| 269 | + | } | |
| 270 | + | } | |
| @@ -3,6 +3,7 @@ | |||||
| 3 | package workflow | 3 | package workflow | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "reflect" | |||
| 6 | "strings" | 7 | "strings" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | ) | 9 | ) | ||
| @@ -200,3 +201,70 @@ jobs: | |||||
| 200 | } | 201 | } | ||
| 201 | } | 202 | } | ||
| 202 | } | 203 | } | ||
| 204 | + | ||||
| 205 | + | // needsFailedDependency is a cut-down workflow of the recorded scenario of | |||
| 206 | + | // that name (without with-failure, with-success and broken's outputs), plus a | |||
| 207 | + | // made-up diamond job that GitHub did not run. | |||
| 208 | + | const needsFailedDependency = `on: | |||
| 209 | + | push: | |||
| 210 | + | branches: ['scn/**'] | |||
| 211 | + | jobs: | |||
| 212 | + | broken: | |||
| 213 | + | runs-on: ubuntu-24.04 | |||
| 214 | + | steps: | |||
| 215 | + | - run: exit 1 | |||
| 216 | + | plain: | |||
| 217 | + | needs: broken | |||
| 218 | + | runs-on: ubuntu-24.04 | |||
| 219 | + | steps: | |||
| 220 | + | - run: echo SHOULD-NOT-RUN | |||
| 221 | + | with-always: | |||
| 222 | + | needs: broken | |||
| 223 | + | if: always() | |||
| 224 | + | runs-on: ubuntu-24.04 | |||
| 225 | + | steps: | |||
| 226 | + | - run: echo | |||
| 227 | + | chain-after-always: | |||
| 228 | + | needs: with-always | |||
| 229 | + | runs-on: ubuntu-24.04 | |||
| 230 | + | steps: | |||
| 231 | + | - run: echo chain-after-always-runs-because-with-always-succeeded | |||
| 232 | + | diamond: | |||
| 233 | + | needs: [chain-after-always, plain, with-always] | |||
| 234 | + | runs-on: ubuntu-24.04 | |||
| 235 | + | steps: | |||
| 236 | + | - run: echo | |||
| 237 | + | ` | |||
| 238 | + | ||||
| 239 | + | // TestAncestors checks the transitive needs that GitHub's default success() | |||
| 240 | + | // looks at (recorded: needs-failed-dependency, chain-after-always was skipped). | |||
| 241 | + | func TestAncestors(t *testing.T) { | |||
| 242 | + | w, errs := Parse("w.yml", []byte(needsFailedDependency)) | |||
| 243 | + | if len(errs) > 0 { | |||
| 244 | + | t.Fatal(errs) | |||
| 245 | + | } | |||
| 246 | + | for id, want := range map[string][]string{ | |||
| 247 | + | "broken": {}, | |||
| 248 | + | "plain": {"broken"}, | |||
| 249 | + | "with-always": {"broken"}, | |||
| 250 | + | "chain-after-always": {"broken", "with-always"}, | |||
| 251 | + | "diamond": {"broken", "plain", "with-always", "chain-after-always"}, | |||
| 252 | + | "nowhere": nil, | |||
| 253 | + | } { | |||
| 254 | + | if got := w.Ancestors(id); !reflect.DeepEqual(got, want) { | |||
| 255 | + | t.Errorf("Ancestors(%q) = %q, want %q", id, got, want) | |||
| 256 | + | } | |||
| 257 | + | } | |||
| 258 | + | } | |||
| 259 | + | ||||
| 260 | + | // TestAncestorsCycleAndUnknownNeed covers what Parse rejects but a hand-built | |||
| 261 | + | // Workflow may hold: a cycle ends, the start job and unknown ids are left out. | |||
| 262 | + | func TestAncestorsCycleAndUnknownNeed(t *testing.T) { | |||
| 263 | + | w := &Workflow{Jobs: []*Job{ | |||
| 264 | + | {ID: "a", Needs: []string{"b", "ghost"}}, | |||
| 265 | + | {ID: "b", Needs: []string{"a"}}, | |||
| 266 | + | }} | |||
| 267 | + | if got := w.Ancestors("a"); !reflect.DeepEqual(got, []string{"b"}) { | |||
| 268 | + | t.Errorf("Ancestors(a) = %q, want [b]", got) | |||
| 269 | + | } | |||
| 270 | + | } | |||
M workflow/matrix.go +24 -5
| @@ -57,7 +57,8 @@ func (c MatrixCombo) DisplayName(base string) string { | |||
| 57 | 57 | // varying slowest; | |
| 58 | 58 | // - exclude removes every combination that contains all the key-value | |
| 59 | 59 | // pairs of one of its entries; every key of an entry must be a vector of | |
| 60 | - | // the matrix, or the matrix is an error; | |
| 60 | + | // the matrix, or the matrix is an error that matches | |
| 61 | + | // [ErrExcludeUnknownKey]; | |
| 61 | 62 | // - then each include entry is added to every remaining combination whose | |
| 62 | 63 | // values for the vector keys it mentions are equal (an entry that | |
| 63 | 64 | // mentions no vector key matches all), without overwriting the original | |
| @@ -152,10 +153,28 @@ func matrixEntries(name string, v expr.Value) ([]*expr.Dictionary, error) { | |||
| 152 | 153 | return out, nil | |
| 153 | 154 | } | |
| 154 | 155 | ||
| 155 | - | // message is an error whose text is GitHub's, which starts with a capital. | |
| 156 | - | type message string | |
| 156 | + | // ErrExcludeUnknownKey is matched, with [errors.Is], by the error that | |
| 157 | + | // [ExpandMatrix], [ParseStrategy] and [Job.EvaluateStrategy] return when an | |
| 158 | + | // entry of "exclude" names a key that is not a vector of the matrix (keys | |
| 159 | + | // that only "include" adds do not count). GitHub refuses such a workflow as | |
| 160 | + | // invalid, before any job exists, and names the run after the file path | |
| 161 | + | // rather than after the "name:" key (recorded: matrix-edge-values, | |
| 162 | + | // a-exclude-unknown-key.yml). The error carries one line per offending | |
| 163 | + | // entry; when several entries offend, errors.Is still holds. | |
| 164 | + | var ErrExcludeUnknownKey = errors.New("matrix exclude key does not match any key within the matrix") | |
| 157 | 165 | ||
| 158 | - | func (m message) Error() string { return string(m) } | |
| 166 | + | // excludeKeyError is the error for one exclude entry with an unknown key. | |
| 167 | + | type excludeKeyError struct { | |
| 168 | + | // Key is the first unknown key of the entry. | |
| 169 | + | Key string | |
| 170 | + | } | |
| 171 | + | ||
| 172 | + | // Unwrap makes errors.Is(err, [ErrExcludeUnknownKey]) hold. | |
| 173 | + | func (e *excludeKeyError) Unwrap() error { return ErrExcludeUnknownKey } | |
| 174 | + | ||
| 175 | + | func (e *excludeKeyError) Error() string { | |
| 176 | + | return fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", e.Key) | |
| 177 | + | } | |
| 159 | 178 | ||
| 160 | 179 | // errProductTooBig is returned when the cross product of the vectors is too | |
| 161 | 180 | // large to be enumerated. GitHub would count the configurations left after | |
| @@ -236,7 +255,7 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||
| 236 | 255 | vi, ok := index[strings.ToLower(p.Key)] | |
| 237 | 256 | if !ok { | |
| 238 | 257 | // One error per offending entry, for its first unknown key. | |
| 239 | - | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | |
| 258 | + | bad = append(bad, &excludeKeyError{Key: p.Key}) | |
| 240 | 259 | unknown = true | |
| 241 | 260 | break | |
| 242 | 261 | } | |
| @@ -57,7 +57,8 @@ func (c MatrixCombo) DisplayName(base string) string { | |||||
| 57 | // varying slowest; | 57 | // varying slowest; | ||
| 58 | // - exclude removes every combination that contains all the key-value | 58 | // - exclude removes every combination that contains all the key-value | ||
| 59 | // pairs of one of its entries; every key of an entry must be a vector of | 59 | // pairs of one of its entries; every key of an entry must be a vector of | ||
| 60 | - | // the matrix, or the matrix is an error; | 60 | + | // the matrix, or the matrix is an error that matches |
| 61 | + | // [ErrExcludeUnknownKey]; | |||
| 61 | // - then each include entry is added to every remaining combination whose | 62 | // - then each include entry is added to every remaining combination whose | ||
| 62 | // values for the vector keys it mentions are equal (an entry that | 63 | // values for the vector keys it mentions are equal (an entry that | ||
| 63 | // mentions no vector key matches all), without overwriting the original | 64 | // mentions no vector key matches all), without overwriting the original | ||
| @@ -152,10 +153,28 @@ func matrixEntries(name string, v expr.Value) ([]*expr.Dictionary, error) { | |||||
| 152 | return out, nil | 153 | return out, nil | ||
| 153 | } | 154 | } | ||
| 154 | 155 | ||||
| 155 | - | // message is an error whose text is GitHub's, which starts with a capital. | 156 | + | // ErrExcludeUnknownKey is matched, with [errors.Is], by the error that |
| 156 | - | type message string | 157 | + | // [ExpandMatrix], [ParseStrategy] and [Job.EvaluateStrategy] return when an |
| 158 | + | // entry of "exclude" names a key that is not a vector of the matrix (keys | |||
| 159 | + | // that only "include" adds do not count). GitHub refuses such a workflow as | |||
| 160 | + | // invalid, before any job exists, and names the run after the file path | |||
| 161 | + | // rather than after the "name:" key (recorded: matrix-edge-values, | |||
| 162 | + | // a-exclude-unknown-key.yml). The error carries one line per offending | |||
| 163 | + | // entry; when several entries offend, errors.Is still holds. | |||
| 164 | + | var ErrExcludeUnknownKey = errors.New("matrix exclude key does not match any key within the matrix") | |||
| 157 | 165 | ||||
| 158 | - | func (m message) Error() string { return string(m) } | 166 | + | // excludeKeyError is the error for one exclude entry with an unknown key. |
| 167 | + | type excludeKeyError struct { | |||
| 168 | + | // Key is the first unknown key of the entry. | |||
| 169 | + | Key string | |||
| 170 | + | } | |||
| 171 | + | ||||
| 172 | + | // Unwrap makes errors.Is(err, [ErrExcludeUnknownKey]) hold. | |||
| 173 | + | func (e *excludeKeyError) Unwrap() error { return ErrExcludeUnknownKey } | |||
| 174 | + | ||||
| 175 | + | func (e *excludeKeyError) Error() string { | |||
| 176 | + | return fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", e.Key) | |||
| 177 | + | } | |||
| 159 | 178 | ||||
| 160 | // errProductTooBig is returned when the cross product of the vectors is too | 179 | // errProductTooBig is returned when the cross product of the vectors is too | ||
| 161 | // large to be enumerated. GitHub would count the configurations left after | 180 | // large to be enumerated. GitHub would count the configurations left after | ||
| @@ -236,7 +255,7 @@ func expand(ctx context.Context, vecs []vector, includes, excludes []*expr.Dicti | |||||
| 236 | vi, ok := index[strings.ToLower(p.Key)] | 255 | vi, ok := index[strings.ToLower(p.Key)] | ||
| 237 | if !ok { | 256 | if !ok { | ||
| 238 | // One error per offending entry, for its first unknown key. | 257 | // One error per offending entry, for its first unknown key. | ||
| 239 | - | bad = append(bad, message(fmt.Sprintf("Matrix exclude key '%s' does not match any key within the matrix", p.Key))) | 258 | + | bad = append(bad, &excludeKeyError{Key: p.Key}) |
| 240 | unknown = true | 259 | unknown = true | ||
| 241 | break | 260 | break | ||
| 242 | } | 261 | } | ||
M workflow/matrix_test.go +79 -0
| @@ -402,11 +402,90 @@ func TestRecordedExcludeUnknownKey(t *testing.T) { | |||
| 402 | 402 | if err == nil || err.Error() != one+"\n"+one { | |
| 403 | 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | |
| 404 | 404 | } | |
| 405 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 406 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |
| 407 | + | } | |
| 408 | + | if errors.Is(err, ErrTooManyConfigurations) { | |
| 409 | + | t.Errorf("errors.Is(%v, ErrTooManyConfigurations) = true", err) | |
| 410 | + | } | |
| 405 | 411 | // An entry that is fine does not add one. | |
| 406 | 412 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | |
| 407 | 413 | if err == nil || err.Error() != one { | |
| 408 | 414 | t.Errorf("err = %v, want %q", err, one) | |
| 409 | 415 | } | |
| 416 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 417 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |
| 418 | + | } | |
| 419 | + | } | |
| 420 | + | ||
| 421 | + | // TestExcludeUnknownKeySentinelFromJob checks that the sentinel survives | |
| 422 | + | // Job.EvaluateStrategy, which is what a server calls, on the workflow of the | |
| 423 | + | // recorded scenario (matrix-edge-values, a-exclude-unknown-key.yml). The | |
| 424 | + | // workflow parses: GitHub still refuses it, and names the run after the file. | |
| 425 | + | func TestExcludeUnknownKeySentinelFromJob(t *testing.T) { | |
| 426 | + | w, errs := Parse("a.yml", []byte(`name: a-exclude-unknown-key | |
| 427 | + | on: | |
| 428 | + | push: | |
| 429 | + | branches: ['scn/**'] | |
| 430 | + | jobs: | |
| 431 | + | main: | |
| 432 | + | runs-on: ubuntu-24.04 | |
| 433 | + | strategy: | |
| 434 | + | matrix: | |
| 435 | + | os: [one, two] | |
| 436 | + | exclude: | |
| 437 | + | - nokey: x | |
| 438 | + | - os: one | |
| 439 | + | nokey: y | |
| 440 | + | steps: | |
| 441 | + | - run: echo "os=${{ matrix.os }}" | |
| 442 | + | `)) | |
| 443 | + | if len(errs) > 0 { | |
| 444 | + | t.Fatal(errs) | |
| 445 | + | } | |
| 446 | + | _, err := w.Job("main").EvaluateStrategy(ctxbg, template.Env{}) | |
| 447 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 448 | + | t.Fatalf("EvaluateStrategy err = %v, want ErrExcludeUnknownKey", err) | |
| 449 | + | } | |
| 450 | + | // A key that only an include adds is no vector either. | |
| 451 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "include": [{"b": 1}], "exclude": [{"b": 1}]}}`)) | |
| 452 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |
| 453 | + | t.Errorf("exclude of an include-only key: %v", err) | |
| 454 | + | } | |
| 455 | + | // Other matrix errors do not match it. | |
| 456 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "exclude": [{"a": 1}]}}`)) | |
| 457 | + | if err == nil || errors.Is(err, ErrExcludeUnknownKey) { | |
| 458 | + | t.Errorf("all excluded: %v", err) | |
| 459 | + | } | |
| 460 | + | } | |
| 461 | + | ||
| 462 | + | // TestRecordedNullAndEmptyJobIndex is matrix-edge-values b-null-values.yml, | |
| 463 | + | // v: [1, null, "", true, 1.50, "x y"]. The recording has strategy.job-index 1 | |
| 464 | + | // for matrix.v null and 2 for matrix.v "" (jobs 0..5 in the order of the | |
| 465 | + | // vector), which is what this package produces. conformance compare listed the | |
| 466 | + | // two as swapped only because both legs are named "default-name" and it pairs | |
| 467 | + | // equal names by their position in each source's job listing, which GitHub | |
| 468 | + | // does not give in matrix order. So the expansion order is the recorded one and | |
| 469 | + | // this test pins it. | |
| 470 | + | func TestRecordedNullAndEmptyJobIndex(t *testing.T) { | |
| 471 | + | s, err := ParseStrategy(ctxbg, jsonValue(t, `{"matrix": {"v": [1, null, "", true, 1.50, "x y"]}}`)) | |
| 472 | + | if err != nil { | |
| 473 | + | t.Fatal(err) | |
| 474 | + | } | |
| 475 | + | want := []string{`1`, `null`, `""`, `true`, `1.5`, `"x y"`} | |
| 476 | + | if len(s.Combos) != len(want) { | |
| 477 | + | t.Fatalf("%d combinations, want %d", len(s.Combos), len(want)) | |
| 478 | + | } | |
| 479 | + | for i, c := range s.Combos { | |
| 480 | + | v, _ := c.Values.Get("v") | |
| 481 | + | if got := expr.ToJSON(v); got != want[i] { | |
| 482 | + | t.Errorf("combination %d: matrix.v = %s, want %s", i, got, want[i]) | |
| 483 | + | } | |
| 484 | + | idx, _ := s.Context(i).Get("job-index") | |
| 485 | + | if idx != expr.Number(i) { | |
| 486 | + | t.Errorf("combination %d: strategy.job-index = %v", i, expr.ToJSON(idx)) | |
| 487 | + | } | |
| 488 | + | } | |
| 410 | 489 | } | |
| 411 | 490 | ||
| 412 | 491 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | |
| @@ -402,11 +402,90 @@ func TestRecordedExcludeUnknownKey(t *testing.T) { | |||||
| 402 | if err == nil || err.Error() != one+"\n"+one { | 402 | if err == nil || err.Error() != one+"\n"+one { | ||
| 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | 403 | t.Fatalf("err = %v, want the message once per offending entry", err) | ||
| 404 | } | 404 | } | ||
| 405 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 406 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |||
| 407 | + | } | |||
| 408 | + | if errors.Is(err, ErrTooManyConfigurations) { | |||
| 409 | + | t.Errorf("errors.Is(%v, ErrTooManyConfigurations) = true", err) | |||
| 410 | + | } | |||
| 405 | // An entry that is fine does not add one. | 411 | // An entry that is fine does not add one. | ||
| 406 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | 412 | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"os": ["one", "two"], "exclude": [{"os": "one"}, {"nokey": "x"}]}}`)) | ||
| 407 | if err == nil || err.Error() != one { | 413 | if err == nil || err.Error() != one { | ||
| 408 | t.Errorf("err = %v, want %q", err, one) | 414 | t.Errorf("err = %v, want %q", err, one) | ||
| 409 | } | 415 | } | ||
| 416 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 417 | + | t.Errorf("errors.Is(%v, ErrExcludeUnknownKey) = false", err) | |||
| 418 | + | } | |||
| 419 | + | } | |||
| 420 | + | ||||
| 421 | + | // TestExcludeUnknownKeySentinelFromJob checks that the sentinel survives | |||
| 422 | + | // Job.EvaluateStrategy, which is what a server calls, on the workflow of the | |||
| 423 | + | // recorded scenario (matrix-edge-values, a-exclude-unknown-key.yml). The | |||
| 424 | + | // workflow parses: GitHub still refuses it, and names the run after the file. | |||
| 425 | + | func TestExcludeUnknownKeySentinelFromJob(t *testing.T) { | |||
| 426 | + | w, errs := Parse("a.yml", []byte(`name: a-exclude-unknown-key | |||
| 427 | + | on: | |||
| 428 | + | push: | |||
| 429 | + | branches: ['scn/**'] | |||
| 430 | + | jobs: | |||
| 431 | + | main: | |||
| 432 | + | runs-on: ubuntu-24.04 | |||
| 433 | + | strategy: | |||
| 434 | + | matrix: | |||
| 435 | + | os: [one, two] | |||
| 436 | + | exclude: | |||
| 437 | + | - nokey: x | |||
| 438 | + | - os: one | |||
| 439 | + | nokey: y | |||
| 440 | + | steps: | |||
| 441 | + | - run: echo "os=${{ matrix.os }}" | |||
| 442 | + | `)) | |||
| 443 | + | if len(errs) > 0 { | |||
| 444 | + | t.Fatal(errs) | |||
| 445 | + | } | |||
| 446 | + | _, err := w.Job("main").EvaluateStrategy(ctxbg, template.Env{}) | |||
| 447 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 448 | + | t.Fatalf("EvaluateStrategy err = %v, want ErrExcludeUnknownKey", err) | |||
| 449 | + | } | |||
| 450 | + | // A key that only an include adds is no vector either. | |||
| 451 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "include": [{"b": 1}], "exclude": [{"b": 1}]}}`)) | |||
| 452 | + | if !errors.Is(err, ErrExcludeUnknownKey) { | |||
| 453 | + | t.Errorf("exclude of an include-only key: %v", err) | |||
| 454 | + | } | |||
| 455 | + | // Other matrix errors do not match it. | |||
| 456 | + | _, err = ExpandMatrix(ctxbg, jsonValue(t, `{"matrix": {"a": [1], "exclude": [{"a": 1}]}}`)) | |||
| 457 | + | if err == nil || errors.Is(err, ErrExcludeUnknownKey) { | |||
| 458 | + | t.Errorf("all excluded: %v", err) | |||
| 459 | + | } | |||
| 460 | + | } | |||
| 461 | + | ||||
| 462 | + | // TestRecordedNullAndEmptyJobIndex is matrix-edge-values b-null-values.yml, | |||
| 463 | + | // v: [1, null, "", true, 1.50, "x y"]. The recording has strategy.job-index 1 | |||
| 464 | + | // for matrix.v null and 2 for matrix.v "" (jobs 0..5 in the order of the | |||
| 465 | + | // vector), which is what this package produces. conformance compare listed the | |||
| 466 | + | // two as swapped only because both legs are named "default-name" and it pairs | |||
| 467 | + | // equal names by their position in each source's job listing, which GitHub | |||
| 468 | + | // does not give in matrix order. So the expansion order is the recorded one and | |||
| 469 | + | // this test pins it. | |||
| 470 | + | func TestRecordedNullAndEmptyJobIndex(t *testing.T) { | |||
| 471 | + | s, err := ParseStrategy(ctxbg, jsonValue(t, `{"matrix": {"v": [1, null, "", true, 1.50, "x y"]}}`)) | |||
| 472 | + | if err != nil { | |||
| 473 | + | t.Fatal(err) | |||
| 474 | + | } | |||
| 475 | + | want := []string{`1`, `null`, `""`, `true`, `1.5`, `"x y"`} | |||
| 476 | + | if len(s.Combos) != len(want) { | |||
| 477 | + | t.Fatalf("%d combinations, want %d", len(s.Combos), len(want)) | |||
| 478 | + | } | |||
| 479 | + | for i, c := range s.Combos { | |||
| 480 | + | v, _ := c.Values.Get("v") | |||
| 481 | + | if got := expr.ToJSON(v); got != want[i] { | |||
| 482 | + | t.Errorf("combination %d: matrix.v = %s, want %s", i, got, want[i]) | |||
| 483 | + | } | |||
| 484 | + | idx, _ := s.Context(i).Get("job-index") | |||
| 485 | + | if idx != expr.Number(i) { | |||
| 486 | + | t.Errorf("combination %d: strategy.job-index = %v", i, expr.ToJSON(idx)) | |||
| 487 | + | } | |||
| 488 | + | } | |||
| 410 | } | 489 | } | ||
| 411 | 490 | ||||
| 412 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | 491 | // TestRecordedMatrixOver256 is the scenario matrix-over-256: 17 x 16 = 272 | ||