formatter: make JUnit testcase names unique - #2522
Conversation
bendrucker
left a comment
There was a problem hiding this comment.
Sorry for the late review, this should cover errors too, then LGTM
| for i, issue := range issues.Sort() { | ||
| cases[i] = formatter.JUnitTestCase{ | ||
| Name: issue.Rule.Name(), | ||
| Name: uniqueJUnitTestCaseName(issue.Rule.Name(), names), |
There was a problem hiding this comment.
This dedups issue names, but the error testcases appended later (cases = append(cases, errorCases...)) skip it. junitErrors sets Name: diag.Summary directly, so multiple diagnostics sharing a summary (e.g. two "Unsupported argument" errors on different lines) still produce duplicate <testcase> names in the same <testsuite> (#1608).
Consider deduping once over the full cases slice after the errors are appended, rather than rewriting names mid-loop for issues only.
| fmt.Fprint(f.Stdout, string(out)) | ||
| } | ||
|
|
||
| func uniqueJUnitTestCaseName(name string, counts map[string]int) string { |
There was a problem hiding this comment.
Minor: the generated suffix can collide with a real rule name. If rule foo appears twice it becomes foo_2; a distinct rule literally named foo_2 would then also emit foo_2. Contrived for today's rule names (no numeric suffixes), so not worth complicating the code over, but perhaps worth at least a comment.
Fixes #1608.
Summary
The JUnit formatter previously used only the rule name as each lint issue's testcase name, so multiple failures from the same rule produced duplicate testcase names. This keeps the first testcase name unchanged and appends an incrementing suffix to later occurrences in the sorted issue output, for example:
terraform_deprecated_interpolationterraform_deprecated_interpolation_2terraform_deprecated_interpolation_3This intentionally avoids adding source ranges or resource names to testcase names; detailed range context remains in the failure message/body.
Validation
gofmt -w formatter/junit.go formatter/junit_test.gogo test ./formattergit diff --check HEAD~1 HEAD