From 5f83f0d98ee02b80fefa8071e60dfee6c5e190ec Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Wed, 10 Feb 2021 09:50:51 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A7=20Add=20diagnostic=20logging=20whe?= =?UTF-8?q?n=20constructing=20metadata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This approach feels very "brute force." I'd like to look for a way to not have to pass the logger in so many places. 🤔 --- cmd/submit.go | 2 +- metadata/metadata.go | 42 +++++++++++++++++++++------------------ metadata/metadata_test.go | 19 +++++++++++------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/cmd/submit.go b/cmd/submit.go index 7ab756f6..e306619c 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -143,7 +143,7 @@ func (s *Submit) Init(args []string, envs map[string]string) error { // Run packages up the test results and sends them to BuildPulse. It returns the // key that uniquely identifies the uploaded object. func (s *Submit) Run() (string, error) { - meta, err := metadata.NewMetadata(s.version, s.envs, s.commitResolver, time.Now) + meta, err := metadata.NewMetadata(s.version, s.envs, s.commitResolver, time.Now, s.diagnostics) if err != nil { return "", err } diff --git a/metadata/metadata.go b/metadata/metadata.go index b0b718c5..35bc96ea 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -2,7 +2,6 @@ package metadata import ( "fmt" - "os" "regexp" "strconv" "strings" @@ -18,11 +17,16 @@ import ( type Metadata interface { MarshalYAML() (out []byte, err error) - initEnvData(envs map[string]string, resolver CommitResolver) error + initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error initTimestamp(now func() time.Time) initVersionData(version *Version) } +// Logger -- TODO Add docs +type Logger interface { + Printf(format string, v ...interface{}) +} + // AbstractMetadata provides the fields that are common across all Metadata // instances, regardless of the specific CI provider. type AbstractMetadata struct { @@ -45,10 +49,10 @@ type AbstractMetadata struct { TreeSHA string `yaml:":tree,omitempty"` } -func (a *AbstractMetadata) initCommitData(cr CommitResolver, sha string) error { +func (a *AbstractMetadata) initCommitData(cr CommitResolver, sha string, log Logger) error { // Git metadata functionality is experimental. While it's experimental, detect a nil CommitResolver and allow the commit metadata fields to be uploaded with empty values. if cr == nil { - fmt.Fprintf(os.Stderr, "[experimental] no commit resolver available; falling back to commit data from environment\n") + log.Printf("[experimental] no commit resolver available; falling back to commit data from environment\n") a.CommitSHA = sha return nil @@ -57,7 +61,7 @@ func (a *AbstractMetadata) initCommitData(cr CommitResolver, sha string) error { // Git metadata functionality is experimental. While it's experimental, don't let this error prevent the test-reporter from continuing normal operation. Allow the commit metadata fields to be uploaded with empty values. c, err := cr.Lookup(sha) if err != nil { - fmt.Fprintf(os.Stderr, "[experimental] git-based commit lookup unsuccessful; falling back to commit data from environment: %v\n", err) + log.Printf("[experimental] git-based commit lookup unsuccessful; falling back to commit data from environment: %v\n", err) a.CommitSHA = sha return nil @@ -86,7 +90,7 @@ func (a *AbstractMetadata) initVersionData(version *Version) { } // NewMetadata creates a new Metadata instance from the given args. -func NewMetadata(version *Version, envs map[string]string, resolver CommitResolver, now func() time.Time) (Metadata, error) { +func NewMetadata(version *Version, envs map[string]string, resolver CommitResolver, now func() time.Time, log Logger) (Metadata, error) { var m Metadata switch { @@ -106,7 +110,7 @@ func NewMetadata(version *Version, envs map[string]string, resolver CommitResolv return nil, fmt.Errorf("unrecognized environment: system does not appear to be a supported CI provider (Buildkite, CircleCI, GitHub Actions, Jenkins, Semaphore, or Travis CI)") } - if err := m.initEnvData(envs, resolver); err != nil { + if err := m.initEnvData(envs, resolver, log); err != nil { return nil, err } m.initTimestamp(now) @@ -142,12 +146,12 @@ type buildkiteMetadata struct { BuildkiteTag string `env:"BUILDKITE_TAG" yaml:":buildkite_tag,omitempty"` } -func (b *buildkiteMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (b *buildkiteMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(b, env.Options{Environment: envs}); err != nil { return err } - if err := b.initCommitData(resolver, b.BuildkiteCommit); err != nil { + if err := b.initCommitData(resolver, b.BuildkiteCommit, log); err != nil { return err } @@ -199,12 +203,12 @@ type circleMetadata struct { CircleWorkflowID string `env:"CIRCLE_WORKFLOW_ID" yaml:":circle_workflow_id"` } -func (c *circleMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (c *circleMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(c, env.Options{Environment: envs}); err != nil { return err } - if err := c.initCommitData(resolver, c.CircleSHA1); err != nil { + if err := c.initCommitData(resolver, c.CircleSHA1, log); err != nil { return err } @@ -243,12 +247,12 @@ type githubMetadata struct { GithubWorkflow string `env:"GITHUB_WORKFLOW" yaml:":github_workflow"` } -func (g *githubMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (g *githubMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(g, env.Options{Environment: envs}); err != nil { return err } - if err := g.initCommitData(resolver, g.GithubSHA); err != nil { + if err := g.initCommitData(resolver, g.GithubSHA, log); err != nil { return err } @@ -302,12 +306,12 @@ type jenkinsMetadata struct { JenkinsWorkspace string `env:"WORKSPACE" yaml:":jenkins_workspace"` } -func (j *jenkinsMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (j *jenkinsMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(j, env.Options{Environment: envs}); err != nil { return err } - if err := j.initCommitData(resolver, j.GitCommit); err != nil { + if err := j.initCommitData(resolver, j.GitCommit, log); err != nil { return err } @@ -363,12 +367,12 @@ type semaphoreMetadata struct { SemaphoreWorkflowNumber uint `env:"SEMAPHORE_WORKFLOW_NUMBER" yaml:":semaphore_workflow_number"` } -func (s *semaphoreMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (s *semaphoreMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(s, env.Options{Environment: envs}); err != nil { return err } - if err := s.initCommitData(resolver, s.SemaphoreGitSHA); err != nil { + if err := s.initCommitData(resolver, s.SemaphoreGitSHA, log); err != nil { return err } @@ -419,12 +423,12 @@ type travisMetadata struct { TravisTestResult uint `env:"TRAVIS_TEST_RESULT" yaml:":travis_test_result"` } -func (t *travisMetadata) initEnvData(envs map[string]string, resolver CommitResolver) error { +func (t *travisMetadata) initEnvData(envs map[string]string, resolver CommitResolver, log Logger) error { if err := env.Parse(t, env.Options{Environment: envs}); err != nil { return err } - if err := t.initCommitData(resolver, t.TravisCommit); err != nil { + if err := t.initCommitData(resolver, t.TravisCommit, log); err != nil { return err } diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index abb35e48..921120b7 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -176,7 +176,7 @@ func TestNewMetadata(t *testing.T) { }) version := &Version{Number: "v1.2.3", GoOS: "linux"} - meta, err := NewMetadata(version, tt.envs, commitResolverDouble, now) + meta, err := NewMetadata(version, tt.envs, commitResolverDouble, now, &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -187,7 +187,7 @@ func TestNewMetadata(t *testing.T) { } func TestNewMetadata_unsupportedProvider(t *testing.T) { - _, err := NewMetadata(&Version{}, map[string]string{}, newCommitResolverStub(), time.Now) + _, err := NewMetadata(&Version{}, map[string]string{}, newCommitResolverStub(), time.Now, &stubLogger{}) if assert.Error(t, err) { assert.Contains(t, err.Error(), "unrecognized environment") } @@ -260,7 +260,7 @@ func TestNewMetadata_customCheckName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - meta, err := NewMetadata(&Version{}, tt.envs, newCommitResolverStub(), time.Now) + meta, err := NewMetadata(&Version{}, tt.envs, newCommitResolverStub(), time.Now, &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -315,7 +315,7 @@ func Test_buildkiteMetadata_initEnvData_extraFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { meta := buildkiteMetadata{} - err := meta.initEnvData(tt.envs, newCommitResolverStub()) + err := meta.initEnvData(tt.envs, newCommitResolverStub(), &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -359,7 +359,7 @@ func Test_circleMetadata_initEnvData_extraFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { meta := circleMetadata{} - err := meta.initEnvData(tt.envs, newCommitResolverStub()) + err := meta.initEnvData(tt.envs, newCommitResolverStub(), &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -400,7 +400,7 @@ func Test_githubMetadata_initEnvData_refTypes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { meta := githubMetadata{} - err := meta.initEnvData(tt.envs, newCommitResolverStub()) + err := meta.initEnvData(tt.envs, newCommitResolverStub(), &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -449,7 +449,7 @@ func Test_travisMetadata_initEnvData_extraFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { meta := travisMetadata{} - err := meta.initEnvData(tt.envs, newCommitResolverStub()) + err := meta.initEnvData(tt.envs, newCommitResolverStub(), &stubLogger{}) assert.NoError(t, err) yaml, err := meta.MarshalYAML() @@ -492,3 +492,8 @@ func newCommitResolverStub() CommitResolver { return &Commit{}, nil }) } + +// TODO: Look for better way to create a no-op logger for testing +type stubLogger struct{} + +func (s *stubLogger) Printf(format string, v ...interface{}) {}