Skip to content

Commit ffa700c

Browse files
mbgmbg
authored
Merge pull request #21214 from github/mbg/go/diagnostics-unit-tests
Go: Make diagnostics unit-testable and add test for `EmitCannotFindPackages`
2 parents bd8a127 + 45e0a92 commit ffa700c

File tree

6 files changed

+177
-40
lines changed

6 files changed

+177
-40
lines changed

‎go/extractor/diagnostics/BUILD.bazel‎

Lines changed: 8 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go/extractor/diagnostics/diagnostics.go‎

Lines changed: 67 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package diagnostics
33
import (
44
"encoding/json"
55
"fmt"
6-
"log"
6+
"log/slog"
77
"os"
88
"strings"
99
"time"
@@ -56,18 +56,65 @@ type diagnostic struct {
5656
var diagnosticsEmitted, diagnosticsLimit uint = 0, 100
5757
var noDiagnosticDirPrinted bool = false
5858

59-
func emitDiagnostic(sourceid, sourcename, markdownMessage string, severity diagnosticSeverity, visibility *visibilityStruct, location *locationStruct) {
60-
if diagnosticsEmitted < diagnosticsLimit {
61-
diagnosticsEmitted += 1
59+
type DiagnosticsWriter interface {
60+
WriteDiagnostic(d diagnostic)
61+
}
6262

63-
diagnosticDir := os.Getenv("CODEQL_EXTRACTOR_GO_DIAGNOSTIC_DIR")
64-
if diagnosticDir == "" {
65-
if !noDiagnosticDirPrinted {
66-
log.Println("No diagnostic directory set, so not emitting diagnostic")
67-
noDiagnosticDirPrinted = true
68-
}
69-
return
63+
type FileDiagnosticsWriter struct {
64+
diagnosticDir string
65+
}
66+
67+
func (writer *FileDiagnosticsWriter) WriteDiagnostic(d diagnostic) {
68+
if writer == nil {
69+
return
70+
}
71+
72+
content, err := json.Marshal(d)
73+
if err != nil {
74+
slog.Error("Failed to encode diagnostic as JSON", slog.Any("err", err))
75+
return
76+
}
77+
78+
targetFile, err := os.CreateTemp(writer.diagnosticDir, "go-extractor.*.json")
79+
if err != nil {
80+
slog.Error("Failed to create diagnostic file", slog.Any("err", err))
81+
return
82+
}
83+
defer func() {
84+
if err := targetFile.Close(); err != nil {
85+
slog.Error("Failed to close diagnostic file", slog.Any("err", err))
86+
}
87+
}()
88+
89+
_, err = targetFile.Write(content)
90+
if err != nil {
91+
slog.Error("Failed to write to diagnostic file", slog.Any("err", err))
92+
}
93+
}
94+
95+
var DefaultWriter *FileDiagnosticsWriter = nil
96+
97+
func NewFileDiagnosticsWriter() *FileDiagnosticsWriter {
98+
diagnosticDir := os.Getenv("CODEQL_EXTRACTOR_GO_DIAGNOSTIC_DIR")
99+
if diagnosticDir == "" {
100+
if !noDiagnosticDirPrinted {
101+
slog.Warn("No diagnostic directory set, so not emitting diagnostics")
102+
noDiagnosticDirPrinted = true
70103
}
104+
return nil
105+
}
106+
107+
return &FileDiagnosticsWriter{diagnosticDir}
108+
}
109+
110+
func init() {
111+
DefaultWriter = NewFileDiagnosticsWriter()
112+
}
113+
114+
// Emits a diagnostic using the specified `DiagnosticsWriter`.
115+
func emitDiagnosticTo(writer DiagnosticsWriter, sourceid, sourcename, markdownMessage string, severity diagnosticSeverity, visibility *visibilityStruct, location *locationStruct) {
116+
if diagnosticsEmitted < diagnosticsLimit {
117+
diagnosticsEmitted += 1
71118

72119
timestamp := time.Now().UTC().Format("2006-01-02T15:04:05.000") + "Z"
73120

@@ -93,33 +140,15 @@ func emitDiagnostic(sourceid, sourcename, markdownMessage string, severity diagn
93140
}
94141
}
95142

96-
content, err := json.Marshal(d)
97-
if err != nil {
98-
log.Println(err)
99-
return
100-
}
101-
102-
targetFile, err := os.CreateTemp(diagnosticDir, "go-extractor.*.json")
103-
if err != nil {
104-
log.Println("Failed to create diagnostic file: ")
105-
log.Println(err)
106-
return
107-
}
108-
defer func() {
109-
if err := targetFile.Close(); err != nil {
110-
log.Println("Failed to close diagnostic file:")
111-
log.Println(err)
112-
}
113-
}()
114-
115-
_, err = targetFile.Write(content)
116-
if err != nil {
117-
log.Println("Failed to write to diagnostic file: ")
118-
log.Println(err)
119-
}
143+
writer.WriteDiagnostic(d)
120144
}
121145
}
122146

147+
// Emits a diagnostic using the default `DiagnosticsWriter`.
148+
func emitDiagnostic(sourceid, sourcename, markdownMessage string, severity diagnosticSeverity, visibility *visibilityStruct, location *locationStruct) {
149+
emitDiagnosticTo(DefaultWriter, sourceid, sourcename, markdownMessage, severity, visibility, location)
150+
}
151+
123152
func EmitPackageDifferentOSArchitecture(pkgPath string) {
124153
emitDiagnostic(
125154
"go/autobuilder/package-different-os-architecture",
@@ -141,7 +170,7 @@ func plural(n int, singular, plural string) string {
141170

142171
const maxNumPkgPaths = 5
143172

144-
func EmitCannotFindPackages(pkgPaths []string) {
173+
func EmitCannotFindPackages(writer DiagnosticsWriter, pkgPaths []string) {
145174
numPkgPaths := len(pkgPaths)
146175

147176
numPrinted := numPkgPaths
@@ -188,7 +217,8 @@ func EmitCannotFindPackages(pkgPaths []string) {
188217
"If any of the packages are already present in the repository, but were not found, then you may need a [custom build command](https://docs.github.com/en/code-security/how-tos/scan-code-for-vulnerabilities/manage-your-configuration/codeql-code-scanning-for-compiled-languages)."
189218
}
190219

191-
emitDiagnostic(
220+
emitDiagnosticTo(
221+
writer,
192222
"go/autobuilder/package-not-found",
193223
"Some packages could not be found",
194224
message,

‎go/extractor/diagnostics/diagnostics_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package diagnostics
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
type memoryDiagnosticsWriter struct {
10+
diagnostics []diagnostic
11+
}
12+
13+
func newMemoryDiagnosticsWriter() *memoryDiagnosticsWriter {
14+
return &memoryDiagnosticsWriter{[]diagnostic{}}
15+
}
16+
17+
func (writer *memoryDiagnosticsWriter) WriteDiagnostic(d diagnostic) {
18+
writer.diagnostics = append(writer.diagnostics, d)
19+
}
20+
21+
func Test_EmitCannotFindPackages_Default(t *testing.T) {
22+
writer := newMemoryDiagnosticsWriter()
23+
24+
// Clear environment variables that affect the diagnostic message.
25+
t.Setenv("GITHUB_EVENT_NAME", "")
26+
t.Setenv("GITHUB_ACTIONS", "")
27+
28+
EmitCannotFindPackages(writer, []string{"github.com/github/foo"})
29+
30+
assert.Len(t, writer.diagnostics, 1, "Expected one diagnostic to be emitted")
31+
32+
d := writer.diagnostics[0]
33+
assert.Equal(t, d.Source.Id, "go/autobuilder/package-not-found")
34+
assert.Equal(t, d.Severity, string(severityWarning))
35+
assert.True(t, d.Visibility.CliSummaryTable)
36+
assert.True(t, d.Visibility.StatusPage)
37+
assert.True(t, d.Visibility.Telemetry)
38+
// Non-Actions suggestion for private registries
39+
assert.Contains(t, d.MarkdownMessage, "ensure that the necessary credentials and environment variables are set up")
40+
// Custom build command suggestion
41+
assert.Contains(t, d.MarkdownMessage, "If any of the packages are already present in the repository")
42+
}
43+
44+
func Test_EmitCannotFindPackages_Dynamic(t *testing.T) {
45+
writer := newMemoryDiagnosticsWriter()
46+
47+
// Set environment variables that affect the diagnostic message.
48+
t.Setenv("GITHUB_EVENT_NAME", "dynamic")
49+
t.Setenv("GITHUB_ACTIONS", "true")
50+
51+
EmitCannotFindPackages(writer, []string{"github.com/github/foo"})
52+
53+
assert.Len(t, writer.diagnostics, 1, "Expected one diagnostic to be emitted")
54+
55+
d := writer.diagnostics[0]
56+
assert.Equal(t, d.Source.Id, "go/autobuilder/package-not-found")
57+
assert.Equal(t, d.Severity, string(severityWarning))
58+
// Dynamic workflow suggestion for private registries
59+
assert.Contains(t, d.MarkdownMessage, "can grant access to private registries for GitHub security products")
60+
// No default suggestions for private registries and custom build command
61+
assert.NotContains(t, d.MarkdownMessage, "ensure that the necessary credentials and environment variables are set up")
62+
assert.NotContains(t, d.MarkdownMessage, "If any of the packages are already present in the repository")
63+
}
64+
65+
func Test_EmitCannotFindPackages_Actions(t *testing.T) {
66+
writer := newMemoryDiagnosticsWriter()
67+
68+
// Set environment variables that affect the diagnostic message.
69+
t.Setenv("GITHUB_EVENT_NAME", "push")
70+
t.Setenv("GITHUB_ACTIONS", "true")
71+
72+
EmitCannotFindPackages(writer, []string{"github.com/github/foo"})
73+
74+
assert.Len(t, writer.diagnostics, 1, "Expected one diagnostic to be emitted")
75+
76+
d := writer.diagnostics[0]
77+
assert.Equal(t, d.Source.Id, "go/autobuilder/package-not-found")
78+
assert.Equal(t, d.Severity, string(severityWarning))
79+
// Advanced workflow suggestion for private registries
80+
assert.Contains(t, d.MarkdownMessage, "add a step to your workflow which sets up")
81+
// No default suggestion for private registries
82+
assert.NotContains(t, d.MarkdownMessage, "ensure that the necessary credentials and environment variables are set up")
83+
// Custom build command suggestion
84+
assert.Contains(t, d.MarkdownMessage, "If any of the packages are already present in the repository")
85+
}

‎go/extractor/extractor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool,
223223
})
224224

225225
if len(pkgsNotFound) > 0 {
226-
diagnostics.EmitCannotFindPackages(pkgsNotFound)
226+
diagnostics.EmitCannotFindPackages(diagnostics.DefaultWriter, pkgsNotFound)
227227
}
228228

229229
for _, pkg := range pkgs {

‎go/extractor/go.mod‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,10 @@ require (
1313
golang.org/x/tools v0.41.0
1414
)
1515

16-
require golang.org/x/sync v0.19.0 // indirect
16+
require (
17+
github.com/davecgh/go-spew v1.1.1 // indirect
18+
github.com/pmezard/go-difflib v1.0.0 // indirect
19+
github.com/stretchr/testify v1.11.1 // indirect
20+
golang.org/x/sync v0.19.0 // indirect
21+
gopkg.in/yaml.v3 v3.0.1 // indirect
22+
)

‎go/extractor/go.sum‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
1+
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
2+
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
13
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
24
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
5+
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
6+
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
7+
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
8+
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
39
golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c=
410
golang.org/x/mod v0.32.0/go.mod h1:SgipZ/3h2Ci89DlEtEXWUk/HteuRin+HHhN+WbNhguU=
511
golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
612
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
713
golang.org/x/tools v0.41.0 h1:a9b8iMweWG+S0OBnlU36rzLp20z1Rp10w+IY2czHTQc=
814
golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg=
15+
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
16+
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
17+
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

0 commit comments

Comments
 (0)