From 3457c98eb1dc93e60c46465e7673ed0e4f156ff6 Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Mon, 25 Jun 2018 15:43:10 -0700 Subject: [PATCH] remove err struct --- plugins/inputs/jenkins/jenkins.go | 75 ++++---------- plugins/inputs/jenkins/jenkins_test.go | 136 ++++++------------------- 2 files changed, 48 insertions(+), 163 deletions(-) diff --git a/plugins/inputs/jenkins/jenkins.go b/plugins/inputs/jenkins/jenkins.go index db6e0f10c..e68e4715b 100644 --- a/plugins/inputs/jenkins/jenkins.go +++ b/plugins/inputs/jenkins/jenkins.go @@ -82,52 +82,10 @@ const ( measurementJob = "jenkins_job" ) -// Error base type of error. -type Error struct { - err error - reference string - url string +func badFormatErr(url string, field interface{}, want string, fieldName string) error { + return fmt.Errorf("error bad format[%s]: fieldName: %s, want %s, got %T", url, fieldName, want, field) } -func newError(err error, ref, url string) *Error { - return &Error{ - err: err, - reference: ref, - url: url, - } -} - -func (e *Error) Error() string { - if e == nil { - return "" - } - return fmt.Sprintf("error %s[%s]: %v", e.reference, e.url, e.err) -} - -func badFormatErr(url string, field interface{}, want string, fieldName string) *Error { - return &Error{ - err: fmt.Errorf("fieldName: %s, want %s, got %T", fieldName, want, field), - reference: errBadFormat, - url: url, - } -} - -// err references -const ( - errParseConfig = "parse jenkins config" - errJobFilterCompile = "compile job filters" - errNodeFilterCompile = "compile node filters" - errConnectJenkins = "connect jenkins instance" - errInitJenkins = "init jenkins instance" - errRetrieveNode = "retrieving nodes" - errRetrieveJobs = "retrieving jobs" - errEmptyNodeName = "empty node name" - errEmptyMonitorData = "empty monitor data" - errBadFormat = "bad format" - errRetrieveInnerJobs = "retrieving inner jobs" - errRetrieveLatestBuild = "retrieving latest build" -) - // SampleConfig implements telegraf.Input interface func (j *Jenkins) SampleConfig() string { return sampleConfig @@ -156,38 +114,39 @@ func (j *Jenkins) Gather(acc telegraf.Accumulator) error { return nil } -func (j *Jenkins) initClient() (*http.Client, *Error) { +func (j *Jenkins) initClient() (*http.Client, error) { tlsCfg, err := j.ClientConfig.TLSConfig() if err != nil { - return nil, newError(err, errParseConfig, j.URL) + return nil, fmt.Errorf("error parse jenkins config[%s]: %v", j.URL, err) } return &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsCfg, + MaxIdleConns: j.MaxConnections, }, Timeout: j.ResponseTimeout.Duration, }, nil } // seperate the client as dependency to use httptest Client for mocking -func (j *Jenkins) newInstance(client *http.Client) *Error { +func (j *Jenkins) newInstance(client *http.Client) error { // create instance var err error j.instance, err = gojenkins.CreateJenkins(client, j.URL, j.Username, j.Password).Init() if err != nil { - return newError(err, errConnectJenkins, j.URL) + return fmt.Errorf("error connect jenkins instance[%s]: %v", j.URL, err) } // init job filter j.jobFilter, err = filter.Compile(j.JobExclude) if err != nil { - return newError(err, errJobFilterCompile, j.URL) + return fmt.Errorf("error compile job filters[%s]: %v", j.URL, err) } // init node filter j.nodeFilter, err = filter.Compile(j.NodeExclude) if err != nil { - return newError(err, errNodeFilterCompile, j.URL) + return fmt.Errorf("error compile node filters[%s]: %v", j.URL, err) } // init tcp pool with default value @@ -203,7 +162,7 @@ func (j *Jenkins) newInstance(client *http.Client) *Error { return nil } -func (j *Jenkins) gatherNodeData(node *gojenkins.Node, url string, acc telegraf.Accumulator) *Error { +func (j *Jenkins) gatherNodeData(node *gojenkins.Node, url string, acc telegraf.Accumulator) error { tags := map[string]string{} fields := make(map[string]interface{}) @@ -211,7 +170,7 @@ func (j *Jenkins) gatherNodeData(node *gojenkins.Node, url string, acc telegraf. // detect the parsing error, since gojenkins lib won't do it if info == nil || info.DisplayName == "" { - return newError(nil, errEmptyNodeName, url) + return fmt.Errorf("error empty node name[%s]: ", j.URL) } tags["node_name"] = info.DisplayName @@ -222,7 +181,7 @@ func (j *Jenkins) gatherNodeData(node *gojenkins.Node, url string, acc telegraf. } if info.MonitorData.Hudson_NodeMonitors_ArchitectureMonitor == nil { - return newError(fmt.Errorf("maybe check your permission"), errEmptyMonitorData, url) + return fmt.Errorf("error empty monitor data[%s]: ", j.URL) } tags["arch"], ok = info.MonitorData.Hudson_NodeMonitors_ArchitectureMonitor.(string) if !ok { @@ -292,7 +251,7 @@ func (j *Jenkins) gatherNodesData(acc telegraf.Accumulator) { // since gojenkins lib will never return error // returns error for len(nodes) is 0 if err != nil || len(nodes) == 0 { - acc.AddError(newError(err, errRetrieveNode, url)) + acc.AddError(fmt.Errorf("error retrieving nodes[%s]: %v", j.URL, err)) return } // get node data @@ -308,7 +267,7 @@ func (j *Jenkins) gatherNodesData(acc telegraf.Accumulator) { func (j *Jenkins) gatherJobs(acc telegraf.Accumulator) { jobs, err := j.instance.GetAllJobNames() if err != nil { - acc.AddError(newError(err, errRetrieveJobs, j.URL)) + acc.AddError(fmt.Errorf("error retrieving jobs[%s]: %v", j.URL, err)) return } jobsC := make(chan jobRequest, j.MaxConnections) @@ -336,7 +295,7 @@ func (j *Jenkins) gatherJobs(acc telegraf.Accumulator) { wg.Wait() } -func (j *Jenkins) getJobDetail(sj jobRequest, jobsC chan<- jobRequest, wg *sync.WaitGroup, acc telegraf.Accumulator) *Error { +func (j *Jenkins) getJobDetail(sj jobRequest, jobsC chan<- jobRequest, wg *sync.WaitGroup, acc telegraf.Accumulator) error { defer wg.Done() if j.MaxSubJobDepth > 0 && sj.layer == j.MaxSubJobDepth { return nil @@ -348,7 +307,7 @@ func (j *Jenkins) getJobDetail(sj jobRequest, jobsC chan<- jobRequest, wg *sync. url := j.URL + "/job/" + strings.Join(sj.combined(), "/job/") + "/api/json" jobDetail, err := j.instance.GetJob(sj.name, sj.parents...) if err != nil { - return newError(err, errRetrieveInnerJobs, url) + return fmt.Errorf("error retrieving inner jobs[%s]: ", url) } for k, innerJob := range jobDetail.Raw.Jobs { @@ -385,7 +344,7 @@ func (j *Jenkins) getJobDetail(sj jobRequest, jobsC chan<- jobRequest, wg *sync. if err == nil && status != 200 { err = fmt.Errorf("status code %d", status) } - return newError(err, errRetrieveLatestBuild, j.URL+baseURL+"/api/json") + return fmt.Errorf("error retrieving inner jobs[%s]: %v", j.URL+baseURL+"/api/json", err) } if build.Raw.Building { diff --git a/plugins/inputs/jenkins/jenkins_test.go b/plugins/inputs/jenkins/jenkins_test.go index 881d5c61d..d71cf10b9 100644 --- a/plugins/inputs/jenkins/jenkins_test.go +++ b/plugins/inputs/jenkins/jenkins_test.go @@ -2,7 +2,6 @@ package jenkins import ( "encoding/json" - "errors" "net/http" "net/http/httptest" "sort" @@ -15,40 +14,6 @@ import ( "github.com/kelwang/gojenkins" ) -func TestErr(t *testing.T) { - tests := []struct { - err *Error - output string - }{ - { - nil, - "", - }, - { - &Error{ - reference: errConnectJenkins, - url: "http://badurl.com", - err: errors.New("unknown error"), - }, - "error connect jenkins instance[http://badurl.com]: unknown error", - }, - { - newError(errors.New("2"), errEmptyMonitorData, "http://badurl.com"), - "error empty monitor data[http://badurl.com]: 2", - }, - { - badFormatErr("http://badurl.com", 20.12, "string", "arch"), - "error bad format[http://badurl.com]: fieldName: arch, want string, got float64", - }, - } - for _, test := range tests { - output := test.err.Error() - if output != test.output { - t.Errorf("Expected %s, got %s\n", test.output, output) - } - } -} - func TestJobRequest(t *testing.T) { tests := []struct { input jobRequest @@ -129,10 +94,10 @@ type monitorData struct { func TestGatherNodeData(t *testing.T) { tests := []struct { - name string - input mockHandler - output *testutil.Accumulator - oe *Error + name string + input mockHandler + output *testutil.Accumulator + wantErr bool }{ { name: "bad endpoint", @@ -142,9 +107,7 @@ func TestGatherNodeData(t *testing.T) { "/computer/api/json": nil, }, }, - oe: &Error{ - reference: errRetrieveNode, - }, + wantErr: true, }, { name: "bad node data", @@ -160,9 +123,7 @@ func TestGatherNodeData(t *testing.T) { }, }, }, - oe: &Error{ - reference: errEmptyNodeName, - }, + wantErr: true, }, { name: "bad empty monitor data", @@ -177,9 +138,7 @@ func TestGatherNodeData(t *testing.T) { }, }, }, - oe: &Error{ - reference: errEmptyMonitorData, - }, + wantErr: true, }, { name: "bad monitor data format", @@ -195,9 +154,7 @@ func TestGatherNodeData(t *testing.T) { }, }, }, - oe: &Error{ - reference: errBadFormat, - }, + wantErr: true, }, { name: "filtered nodes", @@ -286,22 +243,13 @@ func TestGatherNodeData(t *testing.T) { acc := new(testutil.Accumulator) j.gatherNodesData(acc) if err := acc.FirstError(); err != nil { - te = err.(*Error) + te = err } - if test.oe == nil && te != nil { + if !test.wantErr && te != nil { t.Fatalf("%s: failed %s, expected to be nil", test.name, te.Error()) - } else if test.oe != nil { - test.oe.url = ts.URL + "/computer/api/json" - if te == nil { - t.Fatalf("%s: want err: %s, got nil", test.name, test.oe.Error()) - } - if test.oe.reference != te.reference { - t.Fatalf("%s: bad error msg Expected %s, got %s\n", test.name, test.oe.reference, te.reference) - } - if test.oe.url != te.url { - t.Fatalf("%s: bad error url Expected %s, got %s\n", test.name, test.oe.url, te.url) - } + } else if test.wantErr && te == nil { + t.Fatalf("%s: expected err, got nil", test.name) } if test.output == nil && len(acc.Metrics) > 0 { t.Fatalf("%s: collected extra data", test.name) @@ -331,10 +279,10 @@ func TestNewInstance(t *testing.T) { mockClient := &http.Client{Transport: &http.Transport{}} tests := []struct { // name of the test - name string - input *Jenkins - output *Jenkins - oe *Error + name string + input *Jenkins + output *Jenkins + wantErr bool }{ { name: "bad jenkins config", @@ -342,10 +290,7 @@ func TestNewInstance(t *testing.T) { URL: "http://a bad url", ResponseTimeout: internal.Duration{Duration: time.Microsecond}, }, - oe: &Error{ - url: "http://a bad url", - reference: errConnectJenkins, - }, + wantErr: true, }, { name: "has filter", @@ -370,15 +315,10 @@ func TestNewInstance(t *testing.T) { } for _, test := range tests { te := test.input.newInstance(mockClient) - if test.oe == nil && te != nil { + if !test.wantErr && te != nil { t.Fatalf("%s: failed %s, expected to be nil", test.name, te.Error()) - } else if test.oe != nil { - if test.oe.reference != te.reference { - t.Fatalf("%s: bad error msg Expected %s, got %s\n", test.name, test.oe.reference, te.reference) - } - if test.oe.url != te.url { - t.Fatalf("%s: bad error url Expected %s, got %s\n", test.name, test.oe.url, te.url) - } + } else if test.wantErr && te == nil { + t.Fatalf("%s: expected err, got nil", test.name) } if test.output != nil { if test.input.instance == nil { @@ -394,10 +334,10 @@ func TestNewInstance(t *testing.T) { func TestGatherJobs(t *testing.T) { tests := []struct { - name string - input mockHandler - output *testutil.Accumulator - oe *Error + name string + input mockHandler + output *testutil.Accumulator + wantErr bool }{ { name: "empty job", @@ -414,10 +354,7 @@ func TestGatherJobs(t *testing.T) { }, }, }, - oe: &Error{ - reference: errRetrieveInnerJobs, - url: "/job/job1/api/json", - }, + wantErr: true, }, { name: "jobs has no build", @@ -448,10 +385,7 @@ func TestGatherJobs(t *testing.T) { }, }, }, - oe: &Error{ - url: "/job/job1/1/api/json", - reference: errRetrieveLatestBuild, - }, + wantErr: true, }, { name: "ignore building job", @@ -671,22 +605,14 @@ func TestGatherJobs(t *testing.T) { acc := new(testutil.Accumulator) j.gatherJobs(acc) if err := acc.FirstError(); err != nil { - te = err.(*Error) + te = err } - if test.oe == nil && te != nil { + if !test.wantErr && te != nil { t.Fatalf("%s: failed %s, expected to be nil", test.name, te.Error()) - } else if test.oe != nil { - test.oe.url = ts.URL + test.oe.url - if te == nil { - t.Fatalf("%s: want err: %s, got nil", test.name, test.oe.Error()) - } - if test.oe.reference != te.reference { - t.Fatalf("%s: bad error msg Expected %s, got %s\n", test.name, test.oe.reference, te.reference) - } - if test.oe.url != te.url { - t.Fatalf("%s: bad error url Expected %s, got %s\n", test.name, test.oe.url, te.url) - } + } else if test.wantErr && te == nil { + t.Fatalf("%s: expected err, got nil", test.name) } + if test.output != nil && len(test.output.Metrics) > 0 { // sort metrics sort.Slice(acc.Metrics, func(i, j int) bool {