fix(executor): give path-less annotations the line of their step log line
- commit
- 9382a0f83eb0692fe9707665ec33991c50f40898
- author
- brice · · 2026-10-14 12:46 UTC
- committer
- bnema · · 2026-10-14 12:46 UTC
- parent
- ec1337c
- signature
- SSH Unverified
SHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivAThe key is unknown
4 files · +181 -16
M internal/executor/commands_parity_test.go +2 -2
| @@ -65,11 +65,11 @@ func TestInvalidStopTokenFailsTheStepWithTheMessageOnly(t *testing.T) { | |||
| 65 | 65 | func TestAnnotationEndsWhereItStartsByDefault(t *testing.T) { | |
| 66 | 66 | j := &job{} | |
| 67 | 67 | j.masker.Store(newMasker()) | |
| 68 | - | a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m") | |
| 68 | + | a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m", 0) | |
| 69 | 69 | if a.StartLine != 2 || a.EndLine != 2 || a.StartColumn != 3 || a.EndColumn != 3 { | |
| 70 | 70 | t.Errorf("%+v", a) | |
| 71 | 71 | } | |
| 72 | - | a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m") | |
| 72 | + | a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m", 0) | |
| 73 | 73 | if a.EndLine != 5 || a.EndColumn != 0 { | |
| 74 | 74 | t.Errorf("%+v", a) | |
| 75 | 75 | } | |
| @@ -65,11 +65,11 @@ func TestInvalidStopTokenFailsTheStepWithTheMessageOnly(t *testing.T) { | |||||
| 65 | func TestAnnotationEndsWhereItStartsByDefault(t *testing.T) { | 65 | func TestAnnotationEndsWhereItStartsByDefault(t *testing.T) { | ||
| 66 | j := &job{} | 66 | j := &job{} | ||
| 67 | j.masker.Store(newMasker()) | 67 | j.masker.Store(newMasker()) | ||
| 68 | - | a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m") | 68 | + | a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m", 0) |
| 69 | if a.StartLine != 2 || a.EndLine != 2 || a.StartColumn != 3 || a.EndColumn != 3 { | 69 | if a.StartLine != 2 || a.EndLine != 2 || a.StartColumn != 3 || a.EndColumn != 3 { | ||
| 70 | t.Errorf("%+v", a) | 70 | t.Errorf("%+v", a) | ||
| 71 | } | 71 | } | ||
| 72 | - | a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m") | 72 | + | a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m", 0) |
| 73 | if a.EndLine != 5 || a.EndColumn != 0 { | 73 | if a.EndLine != 5 || a.EndColumn != 0 { | ||
| 74 | t.Errorf("%+v", a) | 74 | t.Errorf("%+v", a) | ||
| 75 | } | 75 | } | ||
M internal/executor/issues.go +32 -10
| @@ -4,6 +4,7 @@ package executor | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "fmt" | |
| 7 | + | "math" | |
| 7 | 8 | "strconv" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "sync" | |
| @@ -158,25 +159,31 @@ func (j *job) addIssue(num int, c *issueCollector, embedded bool, is issue) { | |||
| 158 | 159 | if c == nil { | |
| 159 | 160 | c = j.collector(num) | |
| 160 | 161 | } | |
| 161 | - | if msg != "" { | |
| 162 | - | c.add(j.annotationOf(is, msg), embedded) | |
| 163 | - | } | |
| 164 | 162 | logMsg := msg | |
| 165 | 163 | if is.LogText != "" { | |
| 166 | 164 | logMsg = j.mask(is.LogText) | |
| 167 | 165 | } | |
| 168 | - | if logMsg != "" { // an empty message is not logged (:855) | |
| 169 | - | j.logTagged(num, tagOf(is.Level), logMsg) | |
| 166 | + | // logFileLineNumber: the line of the step log that holds the tag, 0 when | |
| 167 | + | // the issue is not logged (an empty message is not, :855). | |
| 168 | + | logLine := 0 | |
| 169 | + | if logMsg != "" { | |
| 170 | + | logLine = j.logTagged(num, tagOf(is.Level), logMsg) | |
| 171 | + | } | |
| 172 | + | if msg != "" { | |
| 173 | + | c.add(j.annotationOf(is, msg, logLine), embedded) | |
| 170 | 174 | } | |
| 171 | 175 | } | |
| 172 | 176 | ||
| 173 | - | // logTagged writes tag+message; only the first line carries the tag. | |
| 174 | - | func (j *job) logTagged(num int, tag, msg string) { | |
| 177 | + | // logTagged writes tag+message; only the first line carries the tag. It | |
| 178 | + | // returns the 1-based number, in the log of the step, of the first line | |
| 179 | + | // (ExecutionContext.Write, :1099-1118). | |
| 180 | + | func (j *job) logTagged(num int, tag, msg string) int { | |
| 175 | 181 | lines := strings.Split(strings.ReplaceAll(msg, "\r\n", "\n"), "\n") | |
| 176 | - | j.write(num, tag+lines[0]) | |
| 182 | + | first := j.write(num, tag+lines[0]) | |
| 177 | 183 | for _, l := range lines[1:] { | |
| 178 | 184 | j.write(num, l) | |
| 179 | 185 | } | |
| 186 | + | return first | |
| 180 | 187 | } | |
| 181 | 188 | ||
| 182 | 189 | // parseInt is int.TryParse(string, out int): white space around, an optional | |
| @@ -196,11 +203,20 @@ func parseInt(s string) (int, bool) { | |||
| 196 | 203 | // Deliberate divergence: GitHub keeps negative and zero lines (int.TryParse, | |
| 197 | 204 | // ActionCommandManager.cs:723-725; ann-levels records start_line -1); ermite's | |
| 198 | 205 | // contract is positive lines, 0 = no line, and the host clamps. | |
| 199 | - | func (j *job) annotationOf(is issue, msg string) Annotation { | |
| 206 | + | // | |
| 207 | + | // logLine is the number of the issue's own line in the step log (0 if it was | |
| 208 | + | // not logged). As in IssueExtensions.ToAnnotation (Sdk/RSWebApi/Contracts/ | |
| 209 | + | // IssueExtensions.cs:25-29), an issue with no file and a line that is 0 or | |
| 210 | + | // absent takes it as start and end line, GitHub's way to point at the log from | |
| 211 | + | // an annotation that has no place in the code. Unlike GitHub, which then puts | |
| 212 | + | // the annotation on a placeholder file ".github", ermite sets no file. | |
| 213 | + | func (j *job) annotationOf(is issue, msg string, logLine int) Annotation { | |
| 200 | 214 | a := Annotation{Level: is.Level, Message: msg} | |
| 201 | 215 | p := is.Props | |
| 202 | 216 | a.Title = truncateRunes(j.mask(p["title"]), maxAnnotationTitle) | |
| 203 | - | if f := j.mask(p["file"]); utf8.RuneCountInString(f) <= maxAnnotationFile { | |
| 217 | + | // A blank file is no file (GitHub: path == null for a blank file), so the | |
| 218 | + | // annotation is path-less and points at its log line below. | |
| 219 | + | if f := j.mask(p["file"]); strings.TrimSpace(f) != "" && utf8.RuneCountInString(f) <= maxAnnotationFile { | |
| 204 | 220 | a.File = f | |
| 205 | 221 | } | |
| 206 | 222 | num := func(k string) int32 { | |
| @@ -222,6 +238,12 @@ func (j *job) annotationOf(is issue, msg string) Annotation { | |||
| 222 | 238 | if a.StartColumn != 0 && a.EndColumn == 0 { | |
| 223 | 239 | a.EndColumn = a.StartColumn | |
| 224 | 240 | } | |
| 241 | + | // path == null && lineNumber == 0 && logLineNumber != 0. The raw line is | |
| 242 | + | // the one that counts: a negative line is not 0 and keeps GitHub's value | |
| 243 | + | // (which ermite cannot send, see above). | |
| 244 | + | if n, ok := parseInt(p["line"]); strings.TrimSpace(p["file"]) == "" && (!ok || n == 0) && logLine > 0 && logLine <= math.MaxInt32 { | |
| 245 | + | a.StartLine, a.EndLine = int32(logLine), int32(logLine) | |
| 246 | + | } | |
| 225 | 247 | return a | |
| 226 | 248 | } | |
| 227 | 249 | ||
| @@ -4,6 +4,7 @@ package executor | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "math" | |||
| 7 | "strconv" | 8 | "strconv" | ||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "sync" | 10 | "sync" | ||
| @@ -158,25 +159,31 @@ func (j *job) addIssue(num int, c *issueCollector, embedded bool, is issue) { | |||||
| 158 | if c == nil { | 159 | if c == nil { | ||
| 159 | c = j.collector(num) | 160 | c = j.collector(num) | ||
| 160 | } | 161 | } | ||
| 161 | - | if msg != "" { | |||
| 162 | - | c.add(j.annotationOf(is, msg), embedded) | |||
| 163 | - | } | |||
| 164 | logMsg := msg | 162 | logMsg := msg | ||
| 165 | if is.LogText != "" { | 163 | if is.LogText != "" { | ||
| 166 | logMsg = j.mask(is.LogText) | 164 | logMsg = j.mask(is.LogText) | ||
| 167 | } | 165 | } | ||
| 168 | - | if logMsg != "" { // an empty message is not logged (:855) | 166 | + | // logFileLineNumber: the line of the step log that holds the tag, 0 when |
| 169 | - | j.logTagged(num, tagOf(is.Level), logMsg) | 167 | + | // the issue is not logged (an empty message is not, :855). |
| 168 | + | logLine := 0 | |||
| 169 | + | if logMsg != "" { | |||
| 170 | + | logLine = j.logTagged(num, tagOf(is.Level), logMsg) | |||
| 171 | + | } | |||
| 172 | + | if msg != "" { | |||
| 173 | + | c.add(j.annotationOf(is, msg, logLine), embedded) | |||
| 170 | } | 174 | } | ||
| 171 | } | 175 | } | ||
| 172 | 176 | ||||
| 173 | - | // logTagged writes tag+message; only the first line carries the tag. | 177 | + | // logTagged writes tag+message; only the first line carries the tag. It |
| 174 | - | func (j *job) logTagged(num int, tag, msg string) { | 178 | + | // returns the 1-based number, in the log of the step, of the first line |
| 179 | + | // (ExecutionContext.Write, :1099-1118). | |||
| 180 | + | func (j *job) logTagged(num int, tag, msg string) int { | |||
| 175 | lines := strings.Split(strings.ReplaceAll(msg, "\r\n", "\n"), "\n") | 181 | lines := strings.Split(strings.ReplaceAll(msg, "\r\n", "\n"), "\n") | ||
| 176 | - | j.write(num, tag+lines[0]) | 182 | + | first := j.write(num, tag+lines[0]) |
| 177 | for _, l := range lines[1:] { | 183 | for _, l := range lines[1:] { | ||
| 178 | j.write(num, l) | 184 | j.write(num, l) | ||
| 179 | } | 185 | } | ||
| 186 | + | return first | |||
| 180 | } | 187 | } | ||
| 181 | 188 | ||||
| 182 | // parseInt is int.TryParse(string, out int): white space around, an optional | 189 | // parseInt is int.TryParse(string, out int): white space around, an optional | ||
| @@ -196,11 +203,20 @@ func parseInt(s string) (int, bool) { | |||||
| 196 | // Deliberate divergence: GitHub keeps negative and zero lines (int.TryParse, | 203 | // Deliberate divergence: GitHub keeps negative and zero lines (int.TryParse, | ||
| 197 | // ActionCommandManager.cs:723-725; ann-levels records start_line -1); ermite's | 204 | // ActionCommandManager.cs:723-725; ann-levels records start_line -1); ermite's | ||
| 198 | // contract is positive lines, 0 = no line, and the host clamps. | 205 | // contract is positive lines, 0 = no line, and the host clamps. | ||
| 199 | - | func (j *job) annotationOf(is issue, msg string) Annotation { | 206 | + | // |
| 207 | + | // logLine is the number of the issue's own line in the step log (0 if it was | |||
| 208 | + | // not logged). As in IssueExtensions.ToAnnotation (Sdk/RSWebApi/Contracts/ | |||
| 209 | + | // IssueExtensions.cs:25-29), an issue with no file and a line that is 0 or | |||
| 210 | + | // absent takes it as start and end line, GitHub's way to point at the log from | |||
| 211 | + | // an annotation that has no place in the code. Unlike GitHub, which then puts | |||
| 212 | + | // the annotation on a placeholder file ".github", ermite sets no file. | |||
| 213 | + | func (j *job) annotationOf(is issue, msg string, logLine int) Annotation { | |||
| 200 | a := Annotation{Level: is.Level, Message: msg} | 214 | a := Annotation{Level: is.Level, Message: msg} | ||
| 201 | p := is.Props | 215 | p := is.Props | ||
| 202 | a.Title = truncateRunes(j.mask(p["title"]), maxAnnotationTitle) | 216 | a.Title = truncateRunes(j.mask(p["title"]), maxAnnotationTitle) | ||
| 203 | - | if f := j.mask(p["file"]); utf8.RuneCountInString(f) <= maxAnnotationFile { | 217 | + | // A blank file is no file (GitHub: path == null for a blank file), so the |
| 218 | + | // annotation is path-less and points at its log line below. | |||
| 219 | + | if f := j.mask(p["file"]); strings.TrimSpace(f) != "" && utf8.RuneCountInString(f) <= maxAnnotationFile { | |||
| 204 | a.File = f | 220 | a.File = f | ||
| 205 | } | 221 | } | ||
| 206 | num := func(k string) int32 { | 222 | num := func(k string) int32 { | ||
| @@ -222,6 +238,12 @@ func (j *job) annotationOf(is issue, msg string) Annotation { | |||||
| 222 | if a.StartColumn != 0 && a.EndColumn == 0 { | 238 | if a.StartColumn != 0 && a.EndColumn == 0 { | ||
| 223 | a.EndColumn = a.StartColumn | 239 | a.EndColumn = a.StartColumn | ||
| 224 | } | 240 | } | ||
| 241 | + | // path == null && lineNumber == 0 && logLineNumber != 0. The raw line is | |||
| 242 | + | // the one that counts: a negative line is not 0 and keeps GitHub's value | |||
| 243 | + | // (which ermite cannot send, see above). | |||
| 244 | + | if n, ok := parseInt(p["line"]); strings.TrimSpace(p["file"]) == "" && (!ok || n == 0) && logLine > 0 && logLine <= math.MaxInt32 { | |||
| 245 | + | a.StartLine, a.EndLine = int32(logLine), int32(logLine) | |||
| 246 | + | } | |||
| 225 | return a | 247 | return a | ||
| 226 | } | 248 | } | ||
| 227 | 249 | ||||
M internal/executor/issues_test.go +132 -2
| @@ -88,8 +88,11 @@ func TestAnnotationsFromCommands(t *testing.T) { | |||
| 88 | 88 | got := rr.results[2].Annotations | |
| 89 | 89 | want := []Annotation{ | |
| 90 | 90 | {Level: LevelWarning, Title: "T", Message: "careful", File: "src/a.go", StartLine: 3, EndLine: 5}, | |
| 91 | - | {Level: LevelFailure, Message: "broken\nline two"}, | |
| 92 | - | {Level: LevelNotice, Message: "fyi"}, | |
| 91 | + | // No file and no line: the line of the ##[error] and ##[notice] lines in | |
| 92 | + | // the step log (header: group, 3 script lines, shell, endgroup = 6 lines; | |
| 93 | + | // the warning is line 7, the error 8 and its second line 9, the notice 10). | |
| 94 | + | {Level: LevelFailure, Message: "broken\nline two", StartLine: 8, EndLine: 8}, | |
| 95 | + | {Level: LevelNotice, Message: "fyi", StartLine: 10, EndLine: 10}, | |
| 93 | 96 | } | |
| 94 | 97 | // col and endColumn are dropped because the lines differ. | |
| 95 | 98 | if fmt.Sprint(got) != fmt.Sprint(want) { | |
| @@ -224,3 +227,130 @@ func TestSaveStateInRunStep(t *testing.T) { | |||
| 224 | 227 | } | |
| 225 | 228 | } | |
| 226 | 229 | } | |
| 230 | + | ||
| 231 | + | // IssueExtensions.ToAnnotation (Sdk/RSWebApi/Contracts/IssueExtensions.cs:25-29): | |
| 232 | + | // an issue without file and line takes the number of its own line in the step | |
| 233 | + | // log (logFileLineNumber, ExecutionContext.cs:855) as start and end line. | |
| 234 | + | func TestAnnotationOfAPathlessIssueIsItsLogLine(t *testing.T) { | |
| 235 | + | j := &job{} | |
| 236 | + | j.masker.Store(newMasker()) | |
| 237 | + | for _, tc := range []struct { | |
| 238 | + | name string | |
| 239 | + | props map[string]string | |
| 240 | + | logLine int | |
| 241 | + | want [2]int32 | |
| 242 | + | }{ | |
| 243 | + | {"no props", nil, 103, [2]int32{103, 103}}, | |
| 244 | + | {"not logged", nil, 0, [2]int32{0, 0}}, | |
| 245 | + | {"blank file", map[string]string{"file": " "}, 5, [2]int32{5, 5}}, | |
| 246 | + | {"zero line", map[string]string{"line": "0"}, 5, [2]int32{5, 5}}, | |
| 247 | + | {"line not a number", map[string]string{"line": "x"}, 5, [2]int32{5, 5}}, | |
| 248 | + | {"a file", map[string]string{"file": "a.go"}, 5, [2]int32{0, 0}}, | |
| 249 | + | {"a line", map[string]string{"line": "2"}, 5, [2]int32{2, 2}}, | |
| 250 | + | {"a line range", map[string]string{"line": "2", "endline": "4"}, 5, [2]int32{2, 4}}, | |
| 251 | + | {"negative line, no file", map[string]string{"line": "-1"}, 5, [2]int32{0, 0}}, | |
| 252 | + | } { | |
| 253 | + | a := j.annotationOf(issue{Level: LevelWarning, Props: tc.props}, "m", tc.logLine) | |
| 254 | + | wantFile := tc.props["file"] | |
| 255 | + | if strings.TrimSpace(wantFile) == "" { | |
| 256 | + | wantFile = "" | |
| 257 | + | } | |
| 258 | + | if got := [2]int32{a.StartLine, a.EndLine}; got != tc.want || a.File != wantFile { | |
| 259 | + | t.Errorf("%s: %+v, want lines %v", tc.name, a, tc.want) | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | } | |
| 263 | + | ||
| 264 | + | // ann-*: a plain ::warning:: gets the number of its line in the log of the step, | |
| 265 | + | // the header counted (recorded: 103 for the exit code error closing a 103 line | |
| 266 | + | // log). Here: group, script (4 lines), shell, endgroup, then the output. | |
| 267 | + | func TestPlainWarningLineIsItsLogLine(t *testing.T) { | |
| 268 | + | needTools(t, "bash") | |
| 269 | + | rr, _ := runResults(t, wfHead+` steps: | |
| 270 | + | - name: s | |
| 271 | + | run: | | |
| 272 | + | echo out | |
| 273 | + | echo "::warning::x" | |
| 274 | + | echo "::warning::x" | |
| 275 | + | exit 3 | |
| 276 | + | `, nil) | |
| 277 | + | log := rr.step(t, "s").Log | |
| 278 | + | var want []int32 | |
| 279 | + | for i, l := range log { | |
| 280 | + | if strings.HasPrefix(l, "##[warning]") || strings.HasPrefix(l, "##[error]") { | |
| 281 | + | want = append(want, int32(i+1)) | |
| 282 | + | } | |
| 283 | + | } | |
| 284 | + | if fmt.Sprint(want) != "[9 10 11]" { | |
| 285 | + | t.Fatalf("log layout changed: %v\n%s", want, strings.Join(log, "\n")) | |
| 286 | + | } | |
| 287 | + | var got []int32 | |
| 288 | + | for _, a := range rr.results[2].Annotations { | |
| 289 | + | if a.File != "" || a.StartLine != a.EndLine { | |
| 290 | + | t.Errorf("%+v", a) | |
| 291 | + | } | |
| 292 | + | got = append(got, a.StartLine) | |
| 293 | + | } | |
| 294 | + | if fmt.Sprint(got) != fmt.Sprint(want) { | |
| 295 | + | t.Errorf("start lines %v, log lines %v\n%s", got, want, strings.Join(log, "\n")) | |
| 296 | + | } | |
| 297 | + | } | |
| 298 | + | ||
| 299 | + | // Mirror of the recording match-catastrophic (GitHub): the annotations of a | |
| 300 | + | // matcher with no file are at the line of their ##[warning] line, 10, 13, 16, | |
| 301 | + | // 18 and 20 in the steps of the corpus script (the removal warning at 14 and, | |
| 302 | + | // in the next step, 7). The step logs are laid out as the recorded ones. | |
| 303 | + | func TestMatcherAnnotationsWithoutFileHaveTheirLogLine(t *testing.T) { | |
| 304 | + | needTools(t, "bash") | |
| 305 | + | rr, res := runResults(t, wfHead+` steps: | |
| 306 | + | - name: workspace as a repository | |
| 307 | + | run: | | |
| 308 | + | mkdir -p src .github | |
| 309 | + | cat > .github/bad.json <<'JSON' | |
| 310 | + | {"problemMatcher":[{"owner":"catastrophic","pattern":[{"regexp":"^((a+)+)$","message":1}]}]} | |
| 311 | + | JSON | |
| 312 | + | cat > .github/good.json <<'JSON' | |
| 313 | + | {"problemMatcher":[{"owner":"good","severity":"warning","pattern":[{"regexp":"^good: (.+)$","message":1}]}]} | |
| 314 | + | JSON | |
| 315 | + | printf 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!\n' > .github/long-line.txt | |
| 316 | + | - name: catastrophic matcher | |
| 317 | + | run: | | |
| 318 | + | echo "::add-matcher::.github/bad.json" | |
| 319 | + | echo "::add-matcher::.github/good.json" | |
| 320 | + | echo "good: before the long line" | |
| 321 | + | cat .github/long-line.txt | |
| 322 | + | echo "good: after the long line" | |
| 323 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!" | |
| 324 | + | echo "good: after a second long line" | |
| 325 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | |
| 326 | + | echo "good: matcher still alive" | |
| 327 | + | - name: the matcher stays removed in the next step | |
| 328 | + | run: | | |
| 329 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!" | |
| 330 | + | echo "aaa" | |
| 331 | + | echo "good: still alive" | |
| 332 | + | `, nil) | |
| 333 | + | if res.Conclusion != ConclusionSuccess { | |
| 334 | + | t.Fatalf("%+v", res) | |
| 335 | + | } | |
| 336 | + | lines := func(num int) (out []string) { | |
| 337 | + | for _, a := range rr.results[num].Annotations { | |
| 338 | + | if a.File != "" || a.StartLine != a.EndLine { | |
| 339 | + | t.Errorf("step %d: %+v", num, a) | |
| 340 | + | } | |
| 341 | + | m, _, _ := strings.Cut(a.Message, ".") | |
| 342 | + | out = append(out, fmt.Sprintf("%d:%s", a.StartLine, m)) | |
| 343 | + | } | |
| 344 | + | return out | |
| 345 | + | } | |
| 346 | + | // The first step has no annotation; the removal message is line 14. | |
| 347 | + | want3 := "[13:before the long line 14:Removing issue matcher 'catastrophic'" + | |
| 348 | + | " 16:after the long line 18:after a second long line 20:matcher still alive]" | |
| 349 | + | if got := fmt.Sprint(lines(3)); got != want3 { | |
| 350 | + | t.Errorf("catastrophic matcher: %v\nwant %v\n%s", got, want3, logOf(rr.step(t, "catastrophic matcher"))) | |
| 351 | + | } | |
| 352 | + | want4 := "[7:Removing issue matcher 'catastrophic' 10:still alive]" | |
| 353 | + | if got := fmt.Sprint(lines(4)); got != want4 { | |
| 354 | + | t.Errorf("next step: %v\nwant %v\n%s", got, want4, logOf(rr.step(t, "the matcher stays removed in the next step"))) | |
| 355 | + | } | |
| 356 | + | } | |
| @@ -88,8 +88,11 @@ func TestAnnotationsFromCommands(t *testing.T) { | |||||
| 88 | got := rr.results[2].Annotations | 88 | got := rr.results[2].Annotations | ||
| 89 | want := []Annotation{ | 89 | want := []Annotation{ | ||
| 90 | {Level: LevelWarning, Title: "T", Message: "careful", File: "src/a.go", StartLine: 3, EndLine: 5}, | 90 | {Level: LevelWarning, Title: "T", Message: "careful", File: "src/a.go", StartLine: 3, EndLine: 5}, | ||
| 91 | - | {Level: LevelFailure, Message: "broken\nline two"}, | 91 | + | // No file and no line: the line of the ##[error] and ##[notice] lines in |
| 92 | - | {Level: LevelNotice, Message: "fyi"}, | 92 | + | // the step log (header: group, 3 script lines, shell, endgroup = 6 lines; |
| 93 | + | // the warning is line 7, the error 8 and its second line 9, the notice 10). | |||
| 94 | + | {Level: LevelFailure, Message: "broken\nline two", StartLine: 8, EndLine: 8}, | |||
| 95 | + | {Level: LevelNotice, Message: "fyi", StartLine: 10, EndLine: 10}, | |||
| 93 | } | 96 | } | ||
| 94 | // col and endColumn are dropped because the lines differ. | 97 | // col and endColumn are dropped because the lines differ. | ||
| 95 | if fmt.Sprint(got) != fmt.Sprint(want) { | 98 | if fmt.Sprint(got) != fmt.Sprint(want) { | ||
| @@ -224,3 +227,130 @@ func TestSaveStateInRunStep(t *testing.T) { | |||||
| 224 | } | 227 | } | ||
| 225 | } | 228 | } | ||
| 226 | } | 229 | } | ||
| 230 | + | ||||
| 231 | + | // IssueExtensions.ToAnnotation (Sdk/RSWebApi/Contracts/IssueExtensions.cs:25-29): | |||
| 232 | + | // an issue without file and line takes the number of its own line in the step | |||
| 233 | + | // log (logFileLineNumber, ExecutionContext.cs:855) as start and end line. | |||
| 234 | + | func TestAnnotationOfAPathlessIssueIsItsLogLine(t *testing.T) { | |||
| 235 | + | j := &job{} | |||
| 236 | + | j.masker.Store(newMasker()) | |||
| 237 | + | for _, tc := range []struct { | |||
| 238 | + | name string | |||
| 239 | + | props map[string]string | |||
| 240 | + | logLine int | |||
| 241 | + | want [2]int32 | |||
| 242 | + | }{ | |||
| 243 | + | {"no props", nil, 103, [2]int32{103, 103}}, | |||
| 244 | + | {"not logged", nil, 0, [2]int32{0, 0}}, | |||
| 245 | + | {"blank file", map[string]string{"file": " "}, 5, [2]int32{5, 5}}, | |||
| 246 | + | {"zero line", map[string]string{"line": "0"}, 5, [2]int32{5, 5}}, | |||
| 247 | + | {"line not a number", map[string]string{"line": "x"}, 5, [2]int32{5, 5}}, | |||
| 248 | + | {"a file", map[string]string{"file": "a.go"}, 5, [2]int32{0, 0}}, | |||
| 249 | + | {"a line", map[string]string{"line": "2"}, 5, [2]int32{2, 2}}, | |||
| 250 | + | {"a line range", map[string]string{"line": "2", "endline": "4"}, 5, [2]int32{2, 4}}, | |||
| 251 | + | {"negative line, no file", map[string]string{"line": "-1"}, 5, [2]int32{0, 0}}, | |||
| 252 | + | } { | |||
| 253 | + | a := j.annotationOf(issue{Level: LevelWarning, Props: tc.props}, "m", tc.logLine) | |||
| 254 | + | wantFile := tc.props["file"] | |||
| 255 | + | if strings.TrimSpace(wantFile) == "" { | |||
| 256 | + | wantFile = "" | |||
| 257 | + | } | |||
| 258 | + | if got := [2]int32{a.StartLine, a.EndLine}; got != tc.want || a.File != wantFile { | |||
| 259 | + | t.Errorf("%s: %+v, want lines %v", tc.name, a, tc.want) | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | } | |||
| 263 | + | ||||
| 264 | + | // ann-*: a plain ::warning:: gets the number of its line in the log of the step, | |||
| 265 | + | // the header counted (recorded: 103 for the exit code error closing a 103 line | |||
| 266 | + | // log). Here: group, script (4 lines), shell, endgroup, then the output. | |||
| 267 | + | func TestPlainWarningLineIsItsLogLine(t *testing.T) { | |||
| 268 | + | needTools(t, "bash") | |||
| 269 | + | rr, _ := runResults(t, wfHead+` steps: | |||
| 270 | + | - name: s | |||
| 271 | + | run: | | |||
| 272 | + | echo out | |||
| 273 | + | echo "::warning::x" | |||
| 274 | + | echo "::warning::x" | |||
| 275 | + | exit 3 | |||
| 276 | + | `, nil) | |||
| 277 | + | log := rr.step(t, "s").Log | |||
| 278 | + | var want []int32 | |||
| 279 | + | for i, l := range log { | |||
| 280 | + | if strings.HasPrefix(l, "##[warning]") || strings.HasPrefix(l, "##[error]") { | |||
| 281 | + | want = append(want, int32(i+1)) | |||
| 282 | + | } | |||
| 283 | + | } | |||
| 284 | + | if fmt.Sprint(want) != "[9 10 11]" { | |||
| 285 | + | t.Fatalf("log layout changed: %v\n%s", want, strings.Join(log, "\n")) | |||
| 286 | + | } | |||
| 287 | + | var got []int32 | |||
| 288 | + | for _, a := range rr.results[2].Annotations { | |||
| 289 | + | if a.File != "" || a.StartLine != a.EndLine { | |||
| 290 | + | t.Errorf("%+v", a) | |||
| 291 | + | } | |||
| 292 | + | got = append(got, a.StartLine) | |||
| 293 | + | } | |||
| 294 | + | if fmt.Sprint(got) != fmt.Sprint(want) { | |||
| 295 | + | t.Errorf("start lines %v, log lines %v\n%s", got, want, strings.Join(log, "\n")) | |||
| 296 | + | } | |||
| 297 | + | } | |||
| 298 | + | ||||
| 299 | + | // Mirror of the recording match-catastrophic (GitHub): the annotations of a | |||
| 300 | + | // matcher with no file are at the line of their ##[warning] line, 10, 13, 16, | |||
| 301 | + | // 18 and 20 in the steps of the corpus script (the removal warning at 14 and, | |||
| 302 | + | // in the next step, 7). The step logs are laid out as the recorded ones. | |||
| 303 | + | func TestMatcherAnnotationsWithoutFileHaveTheirLogLine(t *testing.T) { | |||
| 304 | + | needTools(t, "bash") | |||
| 305 | + | rr, res := runResults(t, wfHead+` steps: | |||
| 306 | + | - name: workspace as a repository | |||
| 307 | + | run: | | |||
| 308 | + | mkdir -p src .github | |||
| 309 | + | cat > .github/bad.json <<'JSON' | |||
| 310 | + | {"problemMatcher":[{"owner":"catastrophic","pattern":[{"regexp":"^((a+)+)$","message":1}]}]} | |||
| 311 | + | JSON | |||
| 312 | + | cat > .github/good.json <<'JSON' | |||
| 313 | + | {"problemMatcher":[{"owner":"good","severity":"warning","pattern":[{"regexp":"^good: (.+)$","message":1}]}]} | |||
| 314 | + | JSON | |||
| 315 | + | printf 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!\n' > .github/long-line.txt | |||
| 316 | + | - name: catastrophic matcher | |||
| 317 | + | run: | | |||
| 318 | + | echo "::add-matcher::.github/bad.json" | |||
| 319 | + | echo "::add-matcher::.github/good.json" | |||
| 320 | + | echo "good: before the long line" | |||
| 321 | + | cat .github/long-line.txt | |||
| 322 | + | echo "good: after the long line" | |||
| 323 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!" | |||
| 324 | + | echo "good: after a second long line" | |||
| 325 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | |||
| 326 | + | echo "good: matcher still alive" | |||
| 327 | + | - name: the matcher stays removed in the next step | |||
| 328 | + | run: | | |||
| 329 | + | echo "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!" | |||
| 330 | + | echo "aaa" | |||
| 331 | + | echo "good: still alive" | |||
| 332 | + | `, nil) | |||
| 333 | + | if res.Conclusion != ConclusionSuccess { | |||
| 334 | + | t.Fatalf("%+v", res) | |||
| 335 | + | } | |||
| 336 | + | lines := func(num int) (out []string) { | |||
| 337 | + | for _, a := range rr.results[num].Annotations { | |||
| 338 | + | if a.File != "" || a.StartLine != a.EndLine { | |||
| 339 | + | t.Errorf("step %d: %+v", num, a) | |||
| 340 | + | } | |||
| 341 | + | m, _, _ := strings.Cut(a.Message, ".") | |||
| 342 | + | out = append(out, fmt.Sprintf("%d:%s", a.StartLine, m)) | |||
| 343 | + | } | |||
| 344 | + | return out | |||
| 345 | + | } | |||
| 346 | + | // The first step has no annotation; the removal message is line 14. | |||
| 347 | + | want3 := "[13:before the long line 14:Removing issue matcher 'catastrophic'" + | |||
| 348 | + | " 16:after the long line 18:after a second long line 20:matcher still alive]" | |||
| 349 | + | if got := fmt.Sprint(lines(3)); got != want3 { | |||
| 350 | + | t.Errorf("catastrophic matcher: %v\nwant %v\n%s", got, want3, logOf(rr.step(t, "catastrophic matcher"))) | |||
| 351 | + | } | |||
| 352 | + | want4 := "[7:Removing issue matcher 'catastrophic' 10:still alive]" | |||
| 353 | + | if got := fmt.Sprint(lines(4)); got != want4 { | |||
| 354 | + | t.Errorf("next step: %v\nwant %v\n%s", got, want4, logOf(rr.step(t, "the matcher stays removed in the next step"))) | |||
| 355 | + | } | |||
| 356 | + | } | |||
M internal/executor/job.go +15 -2
| @@ -178,6 +178,7 @@ type job struct { | |||
| 178 | 178 | collectors map[int]*issueCollector // by step number | |
| 179 | 179 | outcomes map[int]Conclusion | |
| 180 | 180 | summaries map[int]string | |
| 181 | + | logLines map[int]int // lines written to the log of each step, under mu | |
| 181 | 182 | states map[string]*intraState // intra-action state, see saveState | |
| 182 | 183 | } | |
| 183 | 184 | ||
| @@ -305,11 +306,23 @@ func (j *job) timeoutMinutes() int { | |||
| 305 | 306 | ||
| 306 | 307 | // ---------------------------------------------------------------- logging | |
| 307 | 308 | ||
| 308 | - | // write sends one log line to the Reporter, in order. | |
| 309 | - | func (j *job) write(num int, line string) { | |
| 309 | + | // write sends one log line to the Reporter, in order. It returns the 1-based | |
| 310 | + | // number of the line in the log of the step num, what the runner calls | |
| 311 | + | // logFileLineNumber (_logger.TotalLines + 1 in ExecutionContext.Write). The | |
| 312 | + | // steps inside a composite action write to the log of the step that uses it, | |
| 313 | + | // so they count in the same sequence. | |
| 314 | + | func (j *job) write(num int, line string) int { | |
| 310 | 315 | j.mu.Lock() | |
| 311 | 316 | defer j.mu.Unlock() | |
| 317 | + | if j.logLines == nil { | |
| 318 | + | j.logLines = map[int]int{} | |
| 319 | + | } | |
| 320 | + | n := j.logLines[num] + 1 | |
| 321 | + | // A line with a line feed in it is several lines of the log (Logging.cs: | |
| 322 | + | // 90-101); the host splits the text the same way. | |
| 323 | + | j.logLines[num] += 1 + strings.Count(line, "\n") | |
| 312 | 324 | j.cfg.Reporter.Log(num, j.now(), line) | |
| 325 | + | return n | |
| 313 | 326 | } | |
| 314 | 327 | ||
| 315 | 328 | // out writes a message that may hold several lines (ExecutionContext.Output). | |
| @@ -178,6 +178,7 @@ type job struct { | |||||
| 178 | collectors map[int]*issueCollector // by step number | 178 | collectors map[int]*issueCollector // by step number | ||
| 179 | outcomes map[int]Conclusion | 179 | outcomes map[int]Conclusion | ||
| 180 | summaries map[int]string | 180 | summaries map[int]string | ||
| 181 | + | logLines map[int]int // lines written to the log of each step, under mu | |||
| 181 | states map[string]*intraState // intra-action state, see saveState | 182 | states map[string]*intraState // intra-action state, see saveState | ||
| 182 | } | 183 | } | ||
| 183 | 184 | ||||
| @@ -305,11 +306,23 @@ func (j *job) timeoutMinutes() int { | |||||
| 305 | 306 | ||||
| 306 | // ---------------------------------------------------------------- logging | 307 | // ---------------------------------------------------------------- logging | ||
| 307 | 308 | ||||
| 308 | - | // write sends one log line to the Reporter, in order. | 309 | + | // write sends one log line to the Reporter, in order. It returns the 1-based |
| 309 | - | func (j *job) write(num int, line string) { | 310 | + | // number of the line in the log of the step num, what the runner calls |
| 311 | + | // logFileLineNumber (_logger.TotalLines + 1 in ExecutionContext.Write). The | |||
| 312 | + | // steps inside a composite action write to the log of the step that uses it, | |||
| 313 | + | // so they count in the same sequence. | |||
| 314 | + | func (j *job) write(num int, line string) int { | |||
| 310 | j.mu.Lock() | 315 | j.mu.Lock() | ||
| 311 | defer j.mu.Unlock() | 316 | defer j.mu.Unlock() | ||
| 317 | + | if j.logLines == nil { | |||
| 318 | + | j.logLines = map[int]int{} | |||
| 319 | + | } | |||
| 320 | + | n := j.logLines[num] + 1 | |||
| 321 | + | // A line with a line feed in it is several lines of the log (Logging.cs: | |||
| 322 | + | // 90-101); the host splits the text the same way. | |||
| 323 | + | j.logLines[num] += 1 + strings.Count(line, "\n") | |||
| 312 | j.cfg.Reporter.Log(num, j.now(), line) | 324 | j.cfg.Reporter.Log(num, j.now(), line) | ||
| 325 | + | return n | |||
| 313 | } | 326 | } | ||
| 314 | 327 | ||||
| 315 | // out writes a message that may hold several lines (ExecutionContext.Output). | 328 | // out writes a message that may hold several lines (ExecutionContext.Output). | ||