refactor(executor): additive mask API, scope and timeout hook points; save-state keeps spelling
For phase F: job.addMask is the one way to add a secret after the start (the masker is never replaced); embeddedScope, newOutputManager, armStepTimeout and stepTimedOut are the lifecycle hooks of a step, inner or not. Summaries of inner steps are appended to the outer step's. IntraActionState is a case-insensitive Dictionary that keeps the first spelling of a name and its insertion order, and ActionRunner sets STATE_<key> from that enumeration: the state is no longer lower-cased.
- commit
- 1955003de1db2c02d5fe50612a89e6047cbb00aa
- author
- brice · · 2026-10-14 02:45 UTC
- parent
- 8a89149
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +181 -68
M internal/executor/job.go +1 -1
| @@ -165,7 +165,7 @@ type job struct { | |||
| 165 | 165 | collectors map[int]*issueCollector // by step number | |
| 166 | 166 | outcomes map[int]Conclusion | |
| 167 | 167 | summaries map[int]string | |
| 168 | - | states map[string]map[string]string // intra-action state, see saveState | |
| 168 | + | states map[string]*intraState // intra-action state, see saveState | |
| 169 | 169 | } | |
| 170 | 170 | ||
| 171 | 171 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | |
| @@ -165,7 +165,7 @@ type job struct { | |||||
| 165 | collectors map[int]*issueCollector // by step number | 165 | collectors map[int]*issueCollector // by step number | ||
| 166 | outcomes map[int]Conclusion | 166 | outcomes map[int]Conclusion | ||
| 167 | summaries map[int]string | 167 | summaries map[int]string | ||
| 168 | - | states map[string]map[string]string // intra-action state, see saveState | 168 | + | states map[string]*intraState // intra-action state, see saveState |
| 169 | } | 169 | } | ||
| 170 | 170 | ||||
| 171 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | 171 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | ||
M internal/executor/results.go +121 -15
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "os" | |
| 10 | 10 | "strings" | |
| 11 | 11 | "sync/atomic" | |
| 12 | + | "time" | |
| 12 | 13 | ||
| 13 | 14 | "bnema.dev/ermite-actions/template" | |
| 14 | 15 | ) | |
| @@ -48,7 +49,19 @@ func (j *job) githubValue(k string) string { | |||
| 48 | 49 | } | |
| 49 | 50 | ||
| 50 | 51 | // AddMask is SecretMasker.AddValue. | |
| 51 | - | func (s *stepRun) AddMask(v string) { s.j.masker.Load().add(v) } | |
| 52 | + | func (s *stepRun) AddMask(v string) { s.j.addMask(v) } | |
| 53 | + | ||
| 54 | + | // addMask registers values with the masker of the job, with their encodings | |
| 55 | + | // (SecretMasker.AddValue). It is the one way to add a secret after the job | |
| 56 | + | // started (add-mask, a stop-commands token, the token of an action archive): it | |
| 57 | + | // only adds, the masker is never replaced, so the text that other goroutines | |
| 58 | + | // mask meanwhile loses no value. | |
| 59 | + | func (j *job) addMask(values ...string) { | |
| 60 | + | m := j.masker.Load() | |
| 61 | + | for _, v := range values { | |
| 62 | + | m.add(v) | |
| 63 | + | } | |
| 64 | + | } | |
| 52 | 65 | ||
| 53 | 66 | // Matchers is the root of the problem matchers of the job. | |
| 54 | 67 | func (s *stepRun) Matchers() *matcherRoot { return s.j.matchers } | |
| @@ -93,34 +106,71 @@ func (s *stepRun) stateKey() string { | |||
| 93 | 106 | return fmt.Sprint(s.num) | |
| 94 | 107 | } | |
| 95 | 108 | ||
| 109 | + | // intraState is IntraActionState: a Dictionary<string, string> with an | |
| 110 | + | // OrdinalIgnoreCase comparer. A name that is already there keeps the spelling | |
| 111 | + | // it was first saved with and takes the new value (the indexer of Dictionary | |
| 112 | + | // replaces the value, not the key), and the entries enumerate in the order | |
| 113 | + | // they were added: ActionRunner.cs:255-258 sets STATE_<key> from that | |
| 114 | + | // enumeration, so the spelling of the first save-state is the one the action | |
| 115 | + | // sees. | |
| 116 | + | type intraState struct { | |
| 117 | + | keys []string // spelling and order of first insertion | |
| 118 | + | vals map[string]string | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | func (st *intraState) set(name, value string) { | |
| 122 | + | if st.vals == nil { | |
| 123 | + | st.vals = map[string]string{} | |
| 124 | + | } | |
| 125 | + | lo := strings.ToLower(name) | |
| 126 | + | if _, ok := st.vals[lo]; !ok { | |
| 127 | + | st.keys = append(st.keys, name) | |
| 128 | + | } | |
| 129 | + | st.vals[lo] = value | |
| 130 | + | } | |
| 131 | + | ||
| 132 | + | // Pair is one saved name (as first spelled) and its value. | |
| 133 | + | func (st *intraState) pairs() []Pair { | |
| 134 | + | out := make([]Pair, 0, len(st.keys)) | |
| 135 | + | for _, k := range st.keys { | |
| 136 | + | out = append(out, Pair{k, st.vals[strings.ToLower(k)]}) | |
| 137 | + | } | |
| 138 | + | return out | |
| 139 | + | } | |
| 140 | + | ||
| 96 | 141 | func (j *job) saveState(key, name, value string) { | |
| 97 | 142 | j.issuesMu.Lock() | |
| 98 | 143 | defer j.issuesMu.Unlock() | |
| 99 | 144 | if j.states == nil { | |
| 100 | - | j.states = map[string]map[string]string{} | |
| 145 | + | j.states = map[string]*intraState{} | |
| 101 | 146 | } | |
| 102 | - | m := j.states[key] | |
| 103 | - | if m == nil { | |
| 104 | - | m = map[string]string{} | |
| 105 | - | j.states[key] = m | |
| 147 | + | st := j.states[key] | |
| 148 | + | if st == nil { | |
| 149 | + | st = &intraState{} | |
| 150 | + | j.states[key] = st | |
| 106 | 151 | } | |
| 107 | - | // IntraActionState is a Dictionary with an OrdinalIgnoreCase comparer. | |
| 108 | - | m[strings.ToLower(name)] = value | |
| 152 | + | st.set(name, value) | |
| 109 | 153 | } | |
| 110 | 154 | ||
| 111 | - | // IntraActionState returns the state saved under key (the number of the step, | |
| 112 | - | // or the stateKey of its scope), with lower-case names. | |
| 113 | - | func (j *job) intraActionState(key string) map[string]string { | |
| 155 | + | // intraActionState returns the state saved under key (the number of the step, | |
| 156 | + | // or the stateKey of its scope) as the runner enumerates it: names as first | |
| 157 | + | // spelled, in the order they were first saved. Phase F sets STATE_<name> from | |
| 158 | + | // it for the pre, main and post stages of the action. | |
| 159 | + | func (j *job) intraActionState(key string) []Pair { | |
| 114 | 160 | j.issuesMu.Lock() | |
| 115 | 161 | defer j.issuesMu.Unlock() | |
| 116 | - | out := map[string]string{} | |
| 117 | - | for k, v := range j.states[key] { | |
| 118 | - | out[k] = v | |
| 162 | + | if st := j.states[key]; st != nil { | |
| 163 | + | return st.pairs() | |
| 119 | 164 | } | |
| 120 | - | return out | |
| 165 | + | return nil | |
| 121 | 166 | } | |
| 122 | 167 | ||
| 123 | 168 | // Summary receives GITHUB_STEP_SUMMARY after the step (see processStepSummary). | |
| 169 | + | // | |
| 170 | + | // The server keeps one summary per step of the workflow. The steps inside a | |
| 171 | + | // composite action each have a summary file in the runner (uploaded under the | |
| 172 | + | // id of the embedded step); here they are concatenated, in order, under the | |
| 173 | + | // step that uses the action. | |
| 124 | 174 | func (s *stepRun) Summary(markdown string) { | |
| 125 | 175 | j := s.j | |
| 126 | 176 | j.issuesMu.Lock() | |
| @@ -128,6 +178,12 @@ func (s *stepRun) Summary(markdown string) { | |||
| 128 | 178 | if j.summaries == nil { | |
| 129 | 179 | j.summaries = map[int]string{} | |
| 130 | 180 | } | |
| 181 | + | if s.scope.embedded { | |
| 182 | + | markdown = j.summaries[s.num] + markdown | |
| 183 | + | if len(markdown) > stepSummaryLimit { | |
| 184 | + | return // over the limit of the contract: the first ones stay | |
| 185 | + | } | |
| 186 | + | } | |
| 131 | 187 | j.summaries[s.num] = markdown | |
| 132 | 188 | } | |
| 133 | 189 | ||
| @@ -244,3 +300,53 @@ func (j *job) stepTimeout(s *stepRun) int { | |||
| 244 | 300 | } | |
| 245 | 301 | return int(f) | |
| 246 | 302 | } | |
| 303 | + | ||
| 304 | + | // ---- hook points for steps that contain steps (phase F) | |
| 305 | + | // | |
| 306 | + | // A step of the workflow is run by runStep; a composite action runs its inner | |
| 307 | + | // steps itself. What an inner step shares with the step that contains it is | |
| 308 | + | // named by a stepScope, and what its lifecycle needs from the commands is: | |
| 309 | + | // | |
| 310 | + | // - embeddedScope(outer, stateKey): the scope of an inner step. Its issues | |
| 311 | + | // go to the collector of the outer step (counted against its limits when | |
| 312 | + | // the outer step completes) and its summary is appended to the outer | |
| 313 | + | // step's; its save-state goes to the state of stateKey. | |
| 314 | + | // - newOutputManager(s, cancel): one per process of a step (inner or not), | |
| 315 | + | // with the cancellation channel the process gets; call dispose when the | |
| 316 | + | // process is over. | |
| 317 | + | // - armStepTimeout / stepTimedOut: the timeout of a step, inner or not, | |
| 318 | + | // around the run of its body (see runStep). | |
| 319 | + | // - addMask: add a secret. | |
| 320 | + | ||
| 321 | + | // embeddedScope is the scope of a step inside the step outer. | |
| 322 | + | func (j *job) embeddedScope(outer *stepRun, stateKey string) stepScope { | |
| 323 | + | return stepScope{issues: j.collector(outer.num), embedded: true, stateKey: stateKey} | |
| 324 | + | } | |
| 325 | + | ||
| 326 | + | // armStepTimeout starts the timeout-minutes timer of the step (StepsRunner. | |
| 327 | + | // RunStepAsync, :280-296). When it fires, the step is marked timed out and | |
| 328 | + | // cancel is called (the same cancellation as a job cancel: SIGINT, SIGTERM, | |
| 329 | + | // SIGKILL). It returns the function that stops the timer and the number of | |
| 330 | + | // minutes, 0 for none. | |
| 331 | + | func (j *job) armStepTimeout(s *stepRun, cancel func()) (stop func(), minutes int) { | |
| 332 | + | minutes = j.stepTimeout(s) | |
| 333 | + | if minutes <= 0 { | |
| 334 | + | return func() {}, 0 | |
| 335 | + | } | |
| 336 | + | t := time.AfterFunc(time.Duration(minutes)*time.Minute+j.cfg.StepTimeoutSkew, func() { | |
| 337 | + | s.timedOut.Store(true) | |
| 338 | + | cancel() | |
| 339 | + | }) | |
| 340 | + | return func() { t.Stop() }, minutes | |
| 341 | + | } | |
| 342 | + | ||
| 343 | + | // stepTimedOut turns the result of a step whose own timer fired (and not the | |
| 344 | + | // job's cancellation) into a failure with the runner's message | |
| 345 | + | // (StepsRunner.cs:323-330). Other results pass through. | |
| 346 | + | func (j *job) stepTimedOut(s *stepRun, r stepResult, minutes int) stepResult { | |
| 347 | + | if r == resCanceled && s.timedOut.Load() && !j.cancelRequested() { | |
| 348 | + | j.errorf(s.num, "%s", stepTimeoutMessage(s.name, minutes)) | |
| 349 | + | return resFailed | |
| 350 | + | } | |
| 351 | + | return r | |
| 352 | + | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "os" | 9 | "os" | ||
| 10 | "strings" | 10 | "strings" | ||
| 11 | "sync/atomic" | 11 | "sync/atomic" | ||
| 12 | + | "time" | |||
| 12 | 13 | ||||
| 13 | "bnema.dev/ermite-actions/template" | 14 | "bnema.dev/ermite-actions/template" | ||
| 14 | ) | 15 | ) | ||
| @@ -48,7 +49,19 @@ func (j *job) githubValue(k string) string { | |||||
| 48 | } | 49 | } | ||
| 49 | 50 | ||||
| 50 | // AddMask is SecretMasker.AddValue. | 51 | // AddMask is SecretMasker.AddValue. | ||
| 51 | - | func (s *stepRun) AddMask(v string) { s.j.masker.Load().add(v) } | 52 | + | func (s *stepRun) AddMask(v string) { s.j.addMask(v) } |
| 53 | + | ||||
| 54 | + | // addMask registers values with the masker of the job, with their encodings | |||
| 55 | + | // (SecretMasker.AddValue). It is the one way to add a secret after the job | |||
| 56 | + | // started (add-mask, a stop-commands token, the token of an action archive): it | |||
| 57 | + | // only adds, the masker is never replaced, so the text that other goroutines | |||
| 58 | + | // mask meanwhile loses no value. | |||
| 59 | + | func (j *job) addMask(values ...string) { | |||
| 60 | + | m := j.masker.Load() | |||
| 61 | + | for _, v := range values { | |||
| 62 | + | m.add(v) | |||
| 63 | + | } | |||
| 64 | + | } | |||
| 52 | 65 | ||||
| 53 | // Matchers is the root of the problem matchers of the job. | 66 | // Matchers is the root of the problem matchers of the job. | ||
| 54 | func (s *stepRun) Matchers() *matcherRoot { return s.j.matchers } | 67 | func (s *stepRun) Matchers() *matcherRoot { return s.j.matchers } | ||
| @@ -93,34 +106,71 @@ func (s *stepRun) stateKey() string { | |||||
| 93 | return fmt.Sprint(s.num) | 106 | return fmt.Sprint(s.num) | ||
| 94 | } | 107 | } | ||
| 95 | 108 | ||||
| 109 | + | // intraState is IntraActionState: a Dictionary<string, string> with an | |||
| 110 | + | // OrdinalIgnoreCase comparer. A name that is already there keeps the spelling | |||
| 111 | + | // it was first saved with and takes the new value (the indexer of Dictionary | |||
| 112 | + | // replaces the value, not the key), and the entries enumerate in the order | |||
| 113 | + | // they were added: ActionRunner.cs:255-258 sets STATE_<key> from that | |||
| 114 | + | // enumeration, so the spelling of the first save-state is the one the action | |||
| 115 | + | // sees. | |||
| 116 | + | type intraState struct { | |||
| 117 | + | keys []string // spelling and order of first insertion | |||
| 118 | + | vals map[string]string | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | func (st *intraState) set(name, value string) { | |||
| 122 | + | if st.vals == nil { | |||
| 123 | + | st.vals = map[string]string{} | |||
| 124 | + | } | |||
| 125 | + | lo := strings.ToLower(name) | |||
| 126 | + | if _, ok := st.vals[lo]; !ok { | |||
| 127 | + | st.keys = append(st.keys, name) | |||
| 128 | + | } | |||
| 129 | + | st.vals[lo] = value | |||
| 130 | + | } | |||
| 131 | + | ||||
| 132 | + | // Pair is one saved name (as first spelled) and its value. | |||
| 133 | + | func (st *intraState) pairs() []Pair { | |||
| 134 | + | out := make([]Pair, 0, len(st.keys)) | |||
| 135 | + | for _, k := range st.keys { | |||
| 136 | + | out = append(out, Pair{k, st.vals[strings.ToLower(k)]}) | |||
| 137 | + | } | |||
| 138 | + | return out | |||
| 139 | + | } | |||
| 140 | + | ||||
| 96 | func (j *job) saveState(key, name, value string) { | 141 | func (j *job) saveState(key, name, value string) { | ||
| 97 | j.issuesMu.Lock() | 142 | j.issuesMu.Lock() | ||
| 98 | defer j.issuesMu.Unlock() | 143 | defer j.issuesMu.Unlock() | ||
| 99 | if j.states == nil { | 144 | if j.states == nil { | ||
| 100 | - | j.states = map[string]map[string]string{} | 145 | + | j.states = map[string]*intraState{} |
| 101 | } | 146 | } | ||
| 102 | - | m := j.states[key] | 147 | + | st := j.states[key] |
| 103 | - | if m == nil { | 148 | + | if st == nil { |
| 104 | - | m = map[string]string{} | 149 | + | st = &intraState{} |
| 105 | - | j.states[key] = m | 150 | + | j.states[key] = st |
| 106 | } | 151 | } | ||
| 107 | - | // IntraActionState is a Dictionary with an OrdinalIgnoreCase comparer. | 152 | + | st.set(name, value) |
| 108 | - | m[strings.ToLower(name)] = value | |||
| 109 | } | 153 | } | ||
| 110 | 154 | ||||
| 111 | - | // IntraActionState returns the state saved under key (the number of the step, | 155 | + | // intraActionState returns the state saved under key (the number of the step, |
| 112 | - | // or the stateKey of its scope), with lower-case names. | 156 | + | // or the stateKey of its scope) as the runner enumerates it: names as first |
| 113 | - | func (j *job) intraActionState(key string) map[string]string { | 157 | + | // spelled, in the order they were first saved. Phase F sets STATE_<name> from |
| 158 | + | // it for the pre, main and post stages of the action. | |||
| 159 | + | func (j *job) intraActionState(key string) []Pair { | |||
| 114 | j.issuesMu.Lock() | 160 | j.issuesMu.Lock() | ||
| 115 | defer j.issuesMu.Unlock() | 161 | defer j.issuesMu.Unlock() | ||
| 116 | - | out := map[string]string{} | 162 | + | if st := j.states[key]; st != nil { |
| 117 | - | for k, v := range j.states[key] { | 163 | + | return st.pairs() |
| 118 | - | out[k] = v | |||
| 119 | } | 164 | } | ||
| 120 | - | return out | 165 | + | return nil |
| 121 | } | 166 | } | ||
| 122 | 167 | ||||
| 123 | // Summary receives GITHUB_STEP_SUMMARY after the step (see processStepSummary). | 168 | // Summary receives GITHUB_STEP_SUMMARY after the step (see processStepSummary). | ||
| 169 | + | // | |||
| 170 | + | // The server keeps one summary per step of the workflow. The steps inside a | |||
| 171 | + | // composite action each have a summary file in the runner (uploaded under the | |||
| 172 | + | // id of the embedded step); here they are concatenated, in order, under the | |||
| 173 | + | // step that uses the action. | |||
| 124 | func (s *stepRun) Summary(markdown string) { | 174 | func (s *stepRun) Summary(markdown string) { | ||
| 125 | j := s.j | 175 | j := s.j | ||
| 126 | j.issuesMu.Lock() | 176 | j.issuesMu.Lock() | ||
| @@ -128,6 +178,12 @@ func (s *stepRun) Summary(markdown string) { | |||||
| 128 | if j.summaries == nil { | 178 | if j.summaries == nil { | ||
| 129 | j.summaries = map[int]string{} | 179 | j.summaries = map[int]string{} | ||
| 130 | } | 180 | } | ||
| 181 | + | if s.scope.embedded { | |||
| 182 | + | markdown = j.summaries[s.num] + markdown | |||
| 183 | + | if len(markdown) > stepSummaryLimit { | |||
| 184 | + | return // over the limit of the contract: the first ones stay | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 131 | j.summaries[s.num] = markdown | 187 | j.summaries[s.num] = markdown | ||
| 132 | } | 188 | } | ||
| 133 | 189 | ||||
| @@ -244,3 +300,53 @@ func (j *job) stepTimeout(s *stepRun) int { | |||||
| 244 | } | 300 | } | ||
| 245 | return int(f) | 301 | return int(f) | ||
| 246 | } | 302 | } | ||
| 303 | + | ||||
| 304 | + | // ---- hook points for steps that contain steps (phase F) | |||
| 305 | + | // | |||
| 306 | + | // A step of the workflow is run by runStep; a composite action runs its inner | |||
| 307 | + | // steps itself. What an inner step shares with the step that contains it is | |||
| 308 | + | // named by a stepScope, and what its lifecycle needs from the commands is: | |||
| 309 | + | // | |||
| 310 | + | // - embeddedScope(outer, stateKey): the scope of an inner step. Its issues | |||
| 311 | + | // go to the collector of the outer step (counted against its limits when | |||
| 312 | + | // the outer step completes) and its summary is appended to the outer | |||
| 313 | + | // step's; its save-state goes to the state of stateKey. | |||
| 314 | + | // - newOutputManager(s, cancel): one per process of a step (inner or not), | |||
| 315 | + | // with the cancellation channel the process gets; call dispose when the | |||
| 316 | + | // process is over. | |||
| 317 | + | // - armStepTimeout / stepTimedOut: the timeout of a step, inner or not, | |||
| 318 | + | // around the run of its body (see runStep). | |||
| 319 | + | // - addMask: add a secret. | |||
| 320 | + | ||||
| 321 | + | // embeddedScope is the scope of a step inside the step outer. | |||
| 322 | + | func (j *job) embeddedScope(outer *stepRun, stateKey string) stepScope { | |||
| 323 | + | return stepScope{issues: j.collector(outer.num), embedded: true, stateKey: stateKey} | |||
| 324 | + | } | |||
| 325 | + | ||||
| 326 | + | // armStepTimeout starts the timeout-minutes timer of the step (StepsRunner. | |||
| 327 | + | // RunStepAsync, :280-296). When it fires, the step is marked timed out and | |||
| 328 | + | // cancel is called (the same cancellation as a job cancel: SIGINT, SIGTERM, | |||
| 329 | + | // SIGKILL). It returns the function that stops the timer and the number of | |||
| 330 | + | // minutes, 0 for none. | |||
| 331 | + | func (j *job) armStepTimeout(s *stepRun, cancel func()) (stop func(), minutes int) { | |||
| 332 | + | minutes = j.stepTimeout(s) | |||
| 333 | + | if minutes <= 0 { | |||
| 334 | + | return func() {}, 0 | |||
| 335 | + | } | |||
| 336 | + | t := time.AfterFunc(time.Duration(minutes)*time.Minute+j.cfg.StepTimeoutSkew, func() { | |||
| 337 | + | s.timedOut.Store(true) | |||
| 338 | + | cancel() | |||
| 339 | + | }) | |||
| 340 | + | return func() { t.Stop() }, minutes | |||
| 341 | + | } | |||
| 342 | + | ||||
| 343 | + | // stepTimedOut turns the result of a step whose own timer fired (and not the | |||
| 344 | + | // job's cancellation) into a failure with the runner's message | |||
| 345 | + | // (StepsRunner.cs:323-330). Other results pass through. | |||
| 346 | + | func (j *job) stepTimedOut(s *stepRun, r stepResult, minutes int) stepResult { | |||
| 347 | + | if r == resCanceled && s.timedOut.Load() && !j.cancelRequested() { | |||
| 348 | + | j.errorf(s.num, "%s", stepTimeoutMessage(s.name, minutes)) | |||
| 349 | + | return resFailed | |||
| 350 | + | } | |||
| 351 | + | return r | |||
| 352 | + | } | |||
M internal/executor/results_test.go +56 -36
| @@ -324,14 +324,17 @@ func TestStepTimeoutExpressionError(t *testing.T) { | |||
| 324 | 324 | } | |
| 325 | 325 | } | |
| 326 | 326 | ||
| 327 | - | // save-state keeps the state per step, for the post phase of an action (F); | |
| 328 | - | // names are case-insensitive (IntraActionState uses OrdinalIgnoreCase). | |
| 329 | - | func TestSaveStateIsKeptPerStep(t *testing.T) { | |
| 327 | + | // save-state keeps the state per action; names are case-insensitive, the | |
| 328 | + | // first spelling is the key the action sees (STATE_<key>, ActionRunner.cs:255) | |
| 329 | + | // and entries keep the order they were first saved in. | |
| 330 | + | func TestSaveStateKeepsTheFirstSpellingAndOrder(t *testing.T) { | |
| 330 | 331 | j := &job{} | |
| 331 | 332 | j.saveState("2", "Foo", "bar") | |
| 333 | + | j.saveState("2", "zed", "1") | |
| 332 | 334 | j.saveState("2", "FOO", "baz") | |
| 333 | 335 | j.saveState("3", "x", "y") | |
| 334 | - | if got := j.intraActionState("2"); len(got) != 1 || got["foo"] != "baz" { | |
| 336 | + | got := j.intraActionState("2") | |
| 337 | + | if fmt.Sprint(got) != fmt.Sprint([]Pair{{"Foo", "baz"}, {"zed", "1"}}) { | |
| 335 | 338 | t.Errorf("%v", got) | |
| 336 | 339 | } | |
| 337 | 340 | if got := j.intraActionState("4"); len(got) != 0 { | |
| @@ -339,38 +342,55 @@ func TestSaveStateIsKeptPerStep(t *testing.T) { | |||
| 339 | 342 | } | |
| 340 | 343 | } | |
| 341 | 344 | ||
| 342 | - | // Several catastrophic matchers and a step timeout: the step ends at its | |
| 343 | - | // timeout, not after every matcher has used its 3 attempts on every line | |
| 344 | - | // (GO-004). Without the cancellation of the matchers the 8 matchers x 3 | |
| 345 | - | // attempts x 1 s of the first line alone hold the step for 24 s. | |
| 346 | - | func TestCatastrophicMatchersDoNotHoldAStepPastItsTimeout(t *testing.T) { | |
| 347 | - | needTools(t, "bash") | |
| 348 | - | var matchers []string | |
| 349 | - | var adds []string | |
| 350 | - | for i := range 8 { | |
| 351 | - | matchers = append(matchers, fmt.Sprintf(`{"owner":"bad%d","pattern":[{"regexp":"^((a+)+)$","message":1}]}`, i)) | |
| 352 | - | } | |
| 353 | - | adds = append(adds, `echo '{"problemMatcher":[`+strings.Join(matchers, ",")+`]}' > bad.json`) | |
| 354 | - | start := time.Now() | |
| 355 | - | rr, _ := runResults(t, wfHead+` steps: | |
| 356 | - | - name: slow lines | |
| 357 | - | timeout-minutes: 1 | |
| 358 | - | run: | | |
| 359 | - | `+adds[0]+` | |
| 360 | - | echo "::add-matcher::bad.json" | |
| 361 | - | for i in 1 2 3 4 5 6; do echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"; done | |
| 362 | - | sleep 30 | |
| 363 | - | `, func(c *Config) { c.StepTimeoutSkew = -(60*time.Second - 1500*time.Millisecond) }) | |
| 364 | - | took := time.Since(start) | |
| 365 | - | if took > 8*time.Second { | |
| 366 | - | t.Errorf("the step overran its timeout: %v", took) | |
| 367 | - | } | |
| 368 | - | log := logOf(rr.step(t, "slow lines")) | |
| 369 | - | if !strings.Contains(log, "The action 'slow lines' has timed out after 1 minutes.") { | |
| 370 | - | t.Errorf("no timeout message:\n%s", log) | |
| 345 | + | // The hook points phase F calls (see results.go): the additive mask API, the | |
| 346 | + | // scope of an inner step of a composite action, the timeout helpers. | |
| 347 | + | func TestAddMaskIsAdditive(t *testing.T) { | |
| 348 | + | j := &job{} | |
| 349 | + | j.masker.Store(newMasker("first-secret")) | |
| 350 | + | before := j.masker.Load() | |
| 351 | + | j.addMask("second-secret", "third-secret") | |
| 352 | + | if j.masker.Load() != before { | |
| 353 | + | t.Error("the masker was replaced") | |
| 354 | + | } | |
| 355 | + | for _, v := range []string{"first-secret", "second-secret", "third-secret"} { | |
| 356 | + | if j.mask("x "+v) != "x ***" { | |
| 357 | + | t.Errorf("%q is not masked", v) | |
| 358 | + | } | |
| 371 | 359 | } | |
| 372 | - | // the lines that were not matched are still logged | |
| 373 | - | if strings.Count(log, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!") < 6 { | |
| 374 | - | t.Errorf("lines lost after the cancellation:\n%s", log) | |
| 360 | + | } | |
| 361 | + | ||
| 362 | + | func TestEmbeddedScopeSharesTheOuterStep(t *testing.T) { | |
| 363 | + | rec := newRecorder() | |
| 364 | + | rec.StepStarted(4, "outer", time.Now()) | |
| 365 | + | j := &job{cfg: Config{Reporter: rec}} | |
| 366 | + | j.masker.Store(newMasker()) | |
| 367 | + | outer := &stepRun{j: j, num: 4} | |
| 368 | + | inner := &stepRun{j: j, num: 4, scope: j.embeddedScope(outer, "inner-1")} | |
| 369 | + | // the outer step has 8 warnings of its own and the inner one 6: ten are | |
| 370 | + | // kept, the outer step's own first | |
| 371 | + | for range 8 { | |
| 372 | + | j.addIssueFor(outer, issue{Level: LevelWarning, Message: "outer"}) | |
| 373 | + | } | |
| 374 | + | for range 6 { | |
| 375 | + | j.addIssueFor(inner, issue{Level: LevelWarning, Message: "inner"}) | |
| 376 | + | } | |
| 377 | + | got := j.collector(4).result() | |
| 378 | + | if len(got) != 10 || got[7].Message != "outer" || got[8].Message != "inner" || got[9].Message != "inner" { | |
| 379 | + | t.Errorf("%d annotations: %+v", len(got), got) | |
| 380 | + | } | |
| 381 | + | // summaries of inner steps are appended to the outer step's | |
| 382 | + | outer.Summary("# outer\n") | |
| 383 | + | inner.Summary("# inner 1\n") | |
| 384 | + | inner.Summary("# inner 2\n") | |
| 385 | + | if j.summaries[4] != "# outer\n# inner 1\n# inner 2\n" { | |
| 386 | + | t.Errorf("%q", j.summaries[4]) | |
| 387 | + | } | |
| 388 | + | // save-state of an inner step goes to the key of its scope | |
| 389 | + | inner.SaveState("Key", "v") | |
| 390 | + | if g := j.intraActionState("inner-1"); len(g) != 1 || g[0].Key != "Key" { | |
| 391 | + | t.Errorf("%v", g) | |
| 392 | + | } | |
| 393 | + | if g := j.intraActionState("4"); len(g) != 0 { | |
| 394 | + | t.Errorf("%v", g) | |
| 375 | 395 | } | |
| 376 | 396 | } | |
| @@ -324,14 +324,17 @@ func TestStepTimeoutExpressionError(t *testing.T) { | |||||
| 324 | } | 324 | } | ||
| 325 | } | 325 | } | ||
| 326 | 326 | ||||
| 327 | - | // save-state keeps the state per step, for the post phase of an action (F); | 327 | + | // save-state keeps the state per action; names are case-insensitive, the |
| 328 | - | // names are case-insensitive (IntraActionState uses OrdinalIgnoreCase). | 328 | + | // first spelling is the key the action sees (STATE_<key>, ActionRunner.cs:255) |
| 329 | - | func TestSaveStateIsKeptPerStep(t *testing.T) { | 329 | + | // and entries keep the order they were first saved in. |
| 330 | + | func TestSaveStateKeepsTheFirstSpellingAndOrder(t *testing.T) { | |||
| 330 | j := &job{} | 331 | j := &job{} | ||
| 331 | j.saveState("2", "Foo", "bar") | 332 | j.saveState("2", "Foo", "bar") | ||
| 333 | + | j.saveState("2", "zed", "1") | |||
| 332 | j.saveState("2", "FOO", "baz") | 334 | j.saveState("2", "FOO", "baz") | ||
| 333 | j.saveState("3", "x", "y") | 335 | j.saveState("3", "x", "y") | ||
| 334 | - | if got := j.intraActionState("2"); len(got) != 1 || got["foo"] != "baz" { | 336 | + | got := j.intraActionState("2") |
| 337 | + | if fmt.Sprint(got) != fmt.Sprint([]Pair{{"Foo", "baz"}, {"zed", "1"}}) { | |||
| 335 | t.Errorf("%v", got) | 338 | t.Errorf("%v", got) | ||
| 336 | } | 339 | } | ||
| 337 | if got := j.intraActionState("4"); len(got) != 0 { | 340 | if got := j.intraActionState("4"); len(got) != 0 { | ||
| @@ -339,38 +342,55 @@ func TestSaveStateIsKeptPerStep(t *testing.T) { | |||||
| 339 | } | 342 | } | ||
| 340 | } | 343 | } | ||
| 341 | 344 | ||||
| 342 | - | // Several catastrophic matchers and a step timeout: the step ends at its | 345 | + | // The hook points phase F calls (see results.go): the additive mask API, the |
| 343 | - | // timeout, not after every matcher has used its 3 attempts on every line | 346 | + | // scope of an inner step of a composite action, the timeout helpers. |
| 344 | - | // (GO-004). Without the cancellation of the matchers the 8 matchers x 3 | 347 | + | func TestAddMaskIsAdditive(t *testing.T) { |
| 345 | - | // attempts x 1 s of the first line alone hold the step for 24 s. | 348 | + | j := &job{} |
| 346 | - | func TestCatastrophicMatchersDoNotHoldAStepPastItsTimeout(t *testing.T) { | 349 | + | j.masker.Store(newMasker("first-secret")) |
| 347 | - | needTools(t, "bash") | 350 | + | before := j.masker.Load() |
| 348 | - | var matchers []string | 351 | + | j.addMask("second-secret", "third-secret") |
| 349 | - | var adds []string | 352 | + | if j.masker.Load() != before { |
| 350 | - | for i := range 8 { | 353 | + | t.Error("the masker was replaced") |
| 351 | - | matchers = append(matchers, fmt.Sprintf(`{"owner":"bad%d","pattern":[{"regexp":"^((a+)+)$","message":1}]}`, i)) | 354 | + | } |
| 352 | - | } | 355 | + | for _, v := range []string{"first-secret", "second-secret", "third-secret"} { |
| 353 | - | adds = append(adds, `echo '{"problemMatcher":[`+strings.Join(matchers, ",")+`]}' > bad.json`) | 356 | + | if j.mask("x "+v) != "x ***" { |
| 354 | - | start := time.Now() | 357 | + | t.Errorf("%q is not masked", v) |
| 355 | - | rr, _ := runResults(t, wfHead+` steps: | 358 | + | } |
| 356 | - | - name: slow lines | |||
| 357 | - | timeout-minutes: 1 | |||
| 358 | - | run: | | |||
| 359 | - | `+adds[0]+` | |||
| 360 | - | echo "::add-matcher::bad.json" | |||
| 361 | - | for i in 1 2 3 4 5 6; do echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!"; done | |||
| 362 | - | sleep 30 | |||
| 363 | - | `, func(c *Config) { c.StepTimeoutSkew = -(60*time.Second - 1500*time.Millisecond) }) | |||
| 364 | - | took := time.Since(start) | |||
| 365 | - | if took > 8*time.Second { | |||
| 366 | - | t.Errorf("the step overran its timeout: %v", took) | |||
| 367 | - | } | |||
| 368 | - | log := logOf(rr.step(t, "slow lines")) | |||
| 369 | - | if !strings.Contains(log, "The action 'slow lines' has timed out after 1 minutes.") { | |||
| 370 | - | t.Errorf("no timeout message:\n%s", log) | |||
| 371 | } | 359 | } | ||
| 372 | - | // the lines that were not matched are still logged | 360 | + | } |
| 373 | - | if strings.Count(log, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!") < 6 { | 361 | + | |
| 374 | - | t.Errorf("lines lost after the cancellation:\n%s", log) | 362 | + | func TestEmbeddedScopeSharesTheOuterStep(t *testing.T) { |
| 363 | + | rec := newRecorder() | |||
| 364 | + | rec.StepStarted(4, "outer", time.Now()) | |||
| 365 | + | j := &job{cfg: Config{Reporter: rec}} | |||
| 366 | + | j.masker.Store(newMasker()) | |||
| 367 | + | outer := &stepRun{j: j, num: 4} | |||
| 368 | + | inner := &stepRun{j: j, num: 4, scope: j.embeddedScope(outer, "inner-1")} | |||
| 369 | + | // the outer step has 8 warnings of its own and the inner one 6: ten are | |||
| 370 | + | // kept, the outer step's own first | |||
| 371 | + | for range 8 { | |||
| 372 | + | j.addIssueFor(outer, issue{Level: LevelWarning, Message: "outer"}) | |||
| 373 | + | } | |||
| 374 | + | for range 6 { | |||
| 375 | + | j.addIssueFor(inner, issue{Level: LevelWarning, Message: "inner"}) | |||
| 376 | + | } | |||
| 377 | + | got := j.collector(4).result() | |||
| 378 | + | if len(got) != 10 || got[7].Message != "outer" || got[8].Message != "inner" || got[9].Message != "inner" { | |||
| 379 | + | t.Errorf("%d annotations: %+v", len(got), got) | |||
| 380 | + | } | |||
| 381 | + | // summaries of inner steps are appended to the outer step's | |||
| 382 | + | outer.Summary("# outer\n") | |||
| 383 | + | inner.Summary("# inner 1\n") | |||
| 384 | + | inner.Summary("# inner 2\n") | |||
| 385 | + | if j.summaries[4] != "# outer\n# inner 1\n# inner 2\n" { | |||
| 386 | + | t.Errorf("%q", j.summaries[4]) | |||
| 387 | + | } | |||
| 388 | + | // save-state of an inner step goes to the key of its scope | |||
| 389 | + | inner.SaveState("Key", "v") | |||
| 390 | + | if g := j.intraActionState("inner-1"); len(g) != 1 || g[0].Key != "Key" { | |||
| 391 | + | t.Errorf("%v", g) | |||
| 392 | + | } | |||
| 393 | + | if g := j.intraActionState("4"); len(g) != 0 { | |||
| 394 | + | t.Errorf("%v", g) | |||
| 375 | } | 395 | } | ||
| 376 | } | 396 | } | ||
M internal/executor/step.go +3 -16
| @@ -8,7 +8,6 @@ import ( | |||
| 8 | 8 | "fmt" | |
| 9 | 9 | "strings" | |
| 10 | 10 | "sync" | |
| 11 | - | "time" | |
| 12 | 11 | ||
| 13 | 12 | "bnema.dev/ermite-actions/expr" | |
| 14 | 13 | "bnema.dev/ermite-actions/template" | |
| @@ -337,25 +336,13 @@ func (j *job) runStep(num int, st *workflow.Step) { | |||
| 337 | 336 | }() | |
| 338 | 337 | } | |
| 339 | 338 | ||
| 340 | - | // timeout-minutes (StepsRunner.RunStepAsync, :280-296). | |
| 341 | - | minutes := j.stepTimeout(s) | |
| 342 | - | if minutes > 0 { | |
| 343 | - | t := time.AfterFunc(time.Duration(minutes)*time.Minute+j.cfg.StepTimeoutSkew, func() { | |
| 344 | - | s.timedOut.Store(true) | |
| 345 | - | closeCancel() | |
| 346 | - | }) | |
| 347 | - | defer t.Stop() | |
| 348 | - | } | |
| 339 | + | stopTimeout, minutes := j.armStepTimeout(s, closeCancel) | |
| 340 | + | defer stopTimeout() | |
| 349 | 341 | ||
| 350 | 342 | r := j.execute(s, stepCancel) | |
| 351 | 343 | close(stopWatch) | |
| 352 | 344 | watch.Wait() | |
| 353 | - | if r == resCanceled && s.timedOut.Load() && !j.cancelRequested() { | |
| 354 | - | // The step's own token fired, not the job's: the step failed with a | |
| 355 | - | // message (StepsRunner.cs:323-330). | |
| 356 | - | j.errorf(num, "%s", stepTimeoutMessage(name, minutes)) | |
| 357 | - | r = resFailed | |
| 358 | - | } | |
| 345 | + | r = j.stepTimedOut(s, r, minutes) | |
| 359 | 346 | ||
| 360 | 347 | // Merge the result of the commands (StepsRunner.cs:347-351). | |
| 361 | 348 | if s.cmdResult != nil { | |
| @@ -8,7 +8,6 @@ import ( | |||||
| 8 | "fmt" | 8 | "fmt" | ||
| 9 | "strings" | 9 | "strings" | ||
| 10 | "sync" | 10 | "sync" | ||
| 11 | - | "time" | |||
| 12 | 11 | ||||
| 13 | "bnema.dev/ermite-actions/expr" | 12 | "bnema.dev/ermite-actions/expr" | ||
| 14 | "bnema.dev/ermite-actions/template" | 13 | "bnema.dev/ermite-actions/template" | ||
| @@ -337,25 +336,13 @@ func (j *job) runStep(num int, st *workflow.Step) { | |||||
| 337 | }() | 336 | }() | ||
| 338 | } | 337 | } | ||
| 339 | 338 | ||||
| 340 | - | // timeout-minutes (StepsRunner.RunStepAsync, :280-296). | 339 | + | stopTimeout, minutes := j.armStepTimeout(s, closeCancel) |
| 341 | - | minutes := j.stepTimeout(s) | 340 | + | defer stopTimeout() |
| 342 | - | if minutes > 0 { | |||
| 343 | - | t := time.AfterFunc(time.Duration(minutes)*time.Minute+j.cfg.StepTimeoutSkew, func() { | |||
| 344 | - | s.timedOut.Store(true) | |||
| 345 | - | closeCancel() | |||
| 346 | - | }) | |||
| 347 | - | defer t.Stop() | |||
| 348 | - | } | |||
| 349 | 341 | ||||
| 350 | r := j.execute(s, stepCancel) | 342 | r := j.execute(s, stepCancel) | ||
| 351 | close(stopWatch) | 343 | close(stopWatch) | ||
| 352 | watch.Wait() | 344 | watch.Wait() | ||
| 353 | - | if r == resCanceled && s.timedOut.Load() && !j.cancelRequested() { | 345 | + | r = j.stepTimedOut(s, r, minutes) |
| 354 | - | // The step's own token fired, not the job's: the step failed with a | |||
| 355 | - | // message (StepsRunner.cs:323-330). | |||
| 356 | - | j.errorf(num, "%s", stepTimeoutMessage(name, minutes)) | |||
| 357 | - | r = resFailed | |||
| 358 | - | } | |||
| 359 | 346 | ||||
| 360 | // Merge the result of the commands (StepsRunner.cs:347-351). | 347 | // Merge the result of the commands (StepsRunner.cs:347-351). | ||
| 361 | if s.cmdResult != nil { | 348 | if s.cmdResult != nil { | ||