From 67315c96f19b401af288f2ddcaa72cbbffb87fcf Mon Sep 17 00:00:00 2001 From: Michael Robinson Date: Sat, 7 Apr 2018 18:55:10 -0600 Subject: [PATCH 1/4] switched from convey to std go subtests --- newznab/newznab_test.go | 254 ++++++++++++++-------------------------- 1 file changed, 87 insertions(+), 167 deletions(-) diff --git a/newznab/newznab_test.go b/newznab/newznab_test.go index 21c90ea..1d40b58 100644 --- a/newznab/newznab_test.go +++ b/newznab/newznab_test.go @@ -1,8 +1,6 @@ package newznab import ( - "crypto/md5" - "encoding/base64" "fmt" "io/ioutil" "net/http" @@ -10,12 +8,11 @@ import ( "regexp" "testing" - log "github.com/Sirupsen/logrus" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/require" ) func TestUsenetCrawlerClient(t *testing.T) { - log.SetLevel(log.DebugLevel) + //log.SetLevel(log.DebugLevel) apiKey := "gibberish" // Set up our mock server @@ -26,8 +23,6 @@ func TestUsenetCrawlerClient(t *testing.T) { reg := regexp.MustCompile(`\W`) fixedPath := reg.ReplaceAllString(r.URL.RawQuery, "_") - log.Info("Local fixture path: tests/fixtures" + r.URL.Path + "/" + fixedPath) - if r.URL.Query()["t"][0] == "get" { // Fetch nzb nzbID := r.URL.Query()["id"][0] @@ -40,7 +35,6 @@ func TestUsenetCrawlerClient(t *testing.T) { } if err != nil { - log.Error(err) w.WriteHeader(http.StatusNotFound) w.Write([]byte("File not found")) } else { @@ -50,203 +44,129 @@ func TestUsenetCrawlerClient(t *testing.T) { defer ts.Close() - Convey("I have setup a torznab client", t, func() { + t.Run("torznab client", func(t *testing.T) { client := New(ts.URL, apiKey, 1234, true) - Convey("I can search using simple query", func() { + t.Run("Simple query search", func(t *testing.T) { categories := []int{CategoryTVHD} results, err := client.SearchWithQuery(categories, "Supernatural S11E01", "tvshows") - //for _, result := range results { - // log.Info(result.JSONString()) - //} - - So(err, ShouldBeNil) - So(len(results), ShouldBeGreaterThan, 0) + require.NoError(t, err) + require.NotEmpty(t, results, "expected results") }) }) - Convey("I have setup a nzb client", t, func() { + t.Run("nzb client", func(t *testing.T) { client := New(ts.URL, apiKey, 1234, false) categories := []int{CategoryTVSD} - Convey("Handle errors", func() { - - Convey("Return an error for an invalid search.", func() { - _, err := client.SearchWithTVDB(categories, 1234, 9, 2) - So(err, ShouldNotBeNil) - }) - - Convey("Return an error for invalid api usage.", func() { - _, err := client.SearchWithTVDB(categories, 5678, 9, 2) - So(err.Error(), ShouldEqual, "newznab api error 100: Invalid API Key") - }) + t.Run("invalid search", func(t *testing.T) { + _, err := client.SearchWithTVDB(categories, 1234, 9, 2) + require.Error(t, err, "expected an error") }) - Convey("When getting TV show information", func() { - - Convey("Given a category and a TheTVDB id", func() { - results, err := client.SearchWithTVDB(categories, 75682, 10, 1) - - Convey("A valid result is returned.", func() { - So(err, ShouldBeNil) - So(len(results), ShouldBeGreaterThan, 0) - }) - }) - - Convey("When given a category and a tvrage id", func() { - results, err := client.SearchWithTVRage(categories, 2870, 10, 1) - - Convey("A valid result is returned.", func() { - So(err, ShouldBeNil) - So(len(results), ShouldBeGreaterThan, 0) - }) - - Convey("I can populate the comments for an NZB.", func() { - nzb := results[1] - So(len(nzb.Comments), ShouldEqual, 0) - So(nzb.NumComments, ShouldBeGreaterThan, 0) - err := client.PopulateComments(&nzb) - So(err, ShouldBeNil) - - for _, comment := range nzb.Comments { - log.Info(comment.JSONString()) - } - - So(len(nzb.Comments), ShouldBeGreaterThan, 0) - }) - - Convey("I can get the download url.", func() { - url := client.NZBDownloadURL(results[0]) - So(len(url), ShouldBeGreaterThan, 0) - log.Infof("URL: %s", url) - }) - - Convey("I can download the NZB.", func() { - bytes, err := client.DownloadNZB(results[0]) - So(err, ShouldBeNil) - - md5Sum := md5.Sum(bytes) - log.WithFields(log.Fields{ - "num_bytes": len(bytes), - "md5": base64.StdEncoding.EncodeToString(md5Sum[:]), - }).Info("downloaded") + t.Run("invalid api usage", func(t *testing.T) { + _, err := client.SearchWithTVDB(categories, 5678, 9, 2) + require.Error(t, err, "expected an error") + require.EqualError(t, err, "newznab api error 100: Invalid API Key") + }) - So(len(bytes), ShouldBeGreaterThan, 0) - }) - }) + t.Run("valid category and TheTVDB id", func(t *testing.T) { + results, err := client.SearchWithTVDB(categories, 75682, 10, 1) + require.NoError(t, err) + require.NotEmpty(t, results, "expected results") }) - Convey("When getting movie information", func() { - Convey("Given multiple categories and an IMDB id", func() { - cats := []int{ - CategoryMovieHD, - CategoryMovieBluRay, + t.Run("valid category and tvrage id", func(t *testing.T) { + results, err := client.SearchWithTVRage(categories, 2870, 10, 1) + require.NoError(t, err) + require.NotEmpty(t, results, "expected results") + + t.Run("populate comments", func(t *testing.T) { + nzb := results[1] + require.Empty(t, nzb.Comments) + require.NotZero(t, nzb.NumComments) + err := client.PopulateComments(&nzb) + require.NoError(t, err) + require.NotEmpty(t, nzb.Comments, "expected at least one comment") + for _, comment := range nzb.Comments { + require.NotEmpty(t, comment, "comment should not be empty") } - results, err := client.SearchWithIMDB(cats, "0371746") - - So(err, ShouldBeNil) - So(len(results), ShouldBeGreaterThan, 0) - - Convey("The results have different categories.", func() { - So(results[0].Category[1], ShouldEqual, "2040") - So(results[22].Category[1], ShouldEqual, "2050") - }) }) - Convey("Given a single category and an IMDB id", func() { - cats := []int{CategoryMovieHD} - results, err := client.SearchWithIMDB(cats, "0364569") - - So(err, ShouldBeNil) - So(len(results), ShouldBeGreaterThan, 0) - - Convey("I can get movie specific fields", func() { - - Convey("An IMDB id.", func() { - imdbAttr := results[0].IMDBID - So(imdbAttr, ShouldEqual, "0364569") - }) - - Convey("An IMDB title.", func() { - imdbAttr := results[0].IMDBTitle - So(imdbAttr, ShouldEqual, "Oldboy") - }) + t.Run("download url", func(t *testing.T) { + url := client.NZBDownloadURL(results[0]) + require.NotEmpty(t, url, "expected a url") + }) - Convey("An IMDB year.", func() { - imdbAttr := results[0].IMDBYear - So(imdbAttr, ShouldEqual, 2003) - }) + t.Run("download nzb", func(t *testing.T) { + bytes, err := client.DownloadNZB(results[0]) + require.NoError(t, err) + require.NotEmpty(t, bytes, "expected to download something") + }) + }) - Convey("An IMDB score.", func() { - imdbAttr := results[0].IMDBScore - So(imdbAttr, ShouldEqual, 8.4) - }) + t.Run("multiple categories and IMDB id", func(t *testing.T) { + cats := []int{ + CategoryMovieHD, + CategoryMovieBluRay, + } + results, err := client.SearchWithIMDB(cats, "0371746") + require.NoError(t, err) + require.NotEmpty(t, results, "expected results") + + require.Equal(t, "2040", results[0].Category[1]) + require.Equal(t, "2050", results[22].Category[1]) + }) - Convey("A cover URL.", func() { - imdbAttr := results[0].CoverURL - So(imdbAttr, ShouldEqual, "https://dognzb.cr/content/covers/movies/thumbs/364569.jpg") - }) - }) + t.Run("single category and IMDB id", func(t *testing.T) { + cats := []int{CategoryMovieHD} + results, err := client.SearchWithIMDB(cats, "0364569") + require.NoError(t, err) + require.NotEmpty(t, results, "expected results") + + t.Run("movie specific fields", func(t *testing.T) { + require.Equal(t, "0364569", results[0].IMDBID) + require.Equal(t, "Oldboy", results[0].IMDBTitle) + require.Equal(t, 2003, results[0].IMDBYear) + require.Equal(t, float32(8.4), results[0].IMDBScore) + require.Equal(t, "https://dognzb.cr/content/covers/movies/thumbs/364569.jpg", results[0].CoverURL) }) }) - Convey("When getting recent items via RSS", func() { + t.Run("recent items via RSS", func(t *testing.T) { num := 50 categories := []int{CategoryMovieAll, CategoryTVAll} - Convey("I can load the current RSS feed.", func() { + t.Run("recent items", func(t *testing.T) { results, err := client.LoadRSSFeed(categories, num) + require.NoError(t, err) + require.Len(t, results, num) + require.Equal(t, "bcdbf3f1e7a1ef964527f1d40d5ec639", results[0].ID) + require.Equal(t, "030517-VSHS0101720WDA20H264V", results[6].Title) - Convey("A valid result is returned.", func() { - So(err, ShouldBeNil) - So(len(results), ShouldEqual, num) + t.Run("airdate with RFC1123Z format", func(t *testing.T) { + require.Equal(t, 2017, results[7].AirDate.Year()) }) - Convey("A TV result is present.", func() { - guid := results[0].ID - So(guid, ShouldEqual, "bcdbf3f1e7a1ef964527f1d40d5ec639") + t.Run("usenetdate with RFC3339 format", func(t *testing.T) { + require.Equal(t, 2017, results[7].UsenetDate.Year()) }) - - Convey("A Movie result is present.", func() { - title := results[6].Title - So(title, ShouldEqual, "030517-VSHS0101720WDA20H264V") - }) - - Convey("An airdate with RFC1123Z format is parsed.", func() { - year := results[7].AirDate.Year() - So(year, ShouldEqual, 2017) - }) - - Convey("An usenetdate with RFC3339 format is parsed.", func() { - year := results[7].UsenetDate.Year() - So(year, ShouldEqual, 2017) - }) - }) - Convey("I can load the RSS feed up to a given NZB ID.", func() { + t.Run("up until", func(t *testing.T) { results, err := client.LoadRSSFeedUntilNZBID(categories, num, "29527a54ac54bb7533abacd7dad66a6a", 0) + require.NoError(t, err) + require.Len(t, results, 101) - Convey("A valid result is returned.", func() { - So(err, ShouldBeNil) - So(len(results), ShouldEqual, 101) - }) - - Convey("Everything up to the given ID is returned.", func() { - firstID := results[0].ID - So(firstID, ShouldEqual, "8841b21c4d2fb96f0d47ca24cae9a5b7") - - lastID := results[len(results)-1].ID - So(lastID, ShouldEqual, "2c6c0e2ac562db69d8b3646deaf2d0cd") + t.Run("boundary results", func(t *testing.T) { + require.Equal(t, "8841b21c4d2fb96f0d47ca24cae9a5b7", results[0].ID) + require.Equal(t, "2c6c0e2ac562db69d8b3646deaf2d0cd", results[len(results)-1].ID) }) - }) - - Convey("I can load the RSS feed up to a given NZB ID but will stop after N tries", func() { - results, err := client.LoadRSSFeedUntilNZBID(categories, num, "does-not-exist", 2) - Convey("100 results with 2 requests were fetched.", func() { - So(err, ShouldBeNil) - So(len(results), ShouldEqual, 100) + t.Run("RSS up until with failures/retries", func(t *testing.T) { + results, err := client.LoadRSSFeedUntilNZBID(categories, num, "does-not-exist", 2) + require.NoError(t, err) + require.Len(t, results, 100) }) }) }) From 6bfe8cf9e84e0263e71805d15c690c4671d67a85 Mon Sep 17 00:00:00 2001 From: Michael Robinson Date: Sat, 7 Apr 2018 18:58:57 -0600 Subject: [PATCH 2/4] fixed govet warnings --- newznab/newznab.go | 2 +- newznab/structs.go | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/newznab/newznab.go b/newznab/newznab.go index f932037..3073ac2 100644 --- a/newznab/newznab.go +++ b/newznab/newznab.go @@ -267,7 +267,7 @@ func (c Client) process(vals url.Values, path string) ([]NZB, error) { log.WithFields(log.Fields{ "name": attr.Name, "value": attr.Value, - }).Debug("encounted unknown attribute") + }).Debug("encontered unknown attribute") } } if nzb.Size == 0 { diff --git a/newznab/structs.go b/newznab/structs.go index 1454d6a..7ecb88d 100644 --- a/newznab/structs.go +++ b/newznab/structs.go @@ -4,6 +4,8 @@ import ( "encoding/json" "encoding/xml" "time" + + "github.com/pkg/errors" ) // NZB represents an NZB found on the index @@ -27,12 +29,12 @@ type NZB struct { Genre string `json:"genre,omitempty"` // TV Specific stuff - TVDBID string `json:"genre,omitempty"` - TVRageID string `json:"genre,omitempty"` + TVDBID string `json:"tvdbid,omitempty"` + TVRageID string `json:"tvrageid,omitempty"` Season string `json:"season,omitempty"` Episode string `json:"episode,omitempty"` TVTitle string `json:"tvtitle,omitempty"` - Rating int `json:"tvtitle,omitempty"` + Rating int `json:"rating,omitempty"` // Movie Specific stuff IMDBID string `json:"imdb,omitempty"` @@ -147,9 +149,15 @@ type Time struct { } func (t *Time) MarshalXML(e *xml.Encoder, start xml.StartElement) error { - e.EncodeToken(start) - e.EncodeToken(xml.CharData([]byte(t.UTC().Format(time.RFC822)))) - e.EncodeToken(xml.EndElement{start.Name}) + if err := e.EncodeToken(start); err != nil { + return errors.Wrap(err, "failed to encode xml token") + } + if err := e.EncodeToken(xml.CharData([]byte(t.UTC().Format(time.RFC822)))); err != nil { + return errors.Wrap(err, "failed to encode xml token") + } + if err := e.EncodeToken(xml.EndElement{Name: start.Name}); err != nil { + return errors.Wrap(err, "failed to encode xml token") + } return nil } From 330b6158944915cdac9835ccff61d1d1a3e0c8b8 Mon Sep 17 00:00:00 2001 From: Michael Robinson Date: Sat, 7 Apr 2018 19:20:07 -0600 Subject: [PATCH 3/4] Added coverage reporting to CI pipeline, added badge for coveralls.io --- .travis.yml | 7 ++++--- README.md | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9e1e14e..8e94bcf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,9 @@ language: go - go: - stable - +before_install: + - go get -u github.com/mattn/goveralls script: - - go test -v -race ./... + - go test -v -race -covermode=count -coverprofile=profile.cov ./... - go vet ./... + - goveralls -coverprofile=provile.cov -service=travis-ci diff --git a/README.md b/README.md index 49fa395..831a560 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,7 @@ [![GoDoc](https://godoc.org/github.com/mrobinsn/go-newznab/newznab?status.svg)](https://godoc.org/github.com/mrobinsn/go-newznab/newznab) [![Go Report Card](https://goreportcard.com/badge/github.com/mrobinsn/go-newznab)](https://goreportcard.com/report/github.com/mrobinsn/go-newznab) [![Build Status](https://travis-ci.org/mrobinsn/go-newznab.svg?branch=master)](https://travis-ci.org/mrobinsn/go-newznab) +[![Coverage Status](https://coveralls.io/repos/github/mrobinsn/go-newznab/badge.svg?branch=master)](https://coveralls.io/github/mrobinsn/go-newznab?branch=master) [![MIT license](http://img.shields.io/badge/license-MIT-brightgreen.svg)](http://opensource.org/licenses/MIT) From 450b93c6bef5837bef00a18ff559346eb6d748c4 Mon Sep 17 00:00:00 2001 From: Michael Robinson Date: Sat, 7 Apr 2018 19:21:29 -0600 Subject: [PATCH 4/4] Improved error handling, removed unnecessary logging. BREAKING CHANGE: Client.NZBDownloadURL now returns an error. --- .travis.yml | 3 +-- newznab/newznab.go | 45 +++++++++++++++++------------------------ newznab/newznab_test.go | 3 ++- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8e94bcf..a6c4d8d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,5 @@ go: before_install: - go get -u github.com/mattn/goveralls script: - - go test -v -race -covermode=count -coverprofile=profile.cov ./... - go vet ./... - - goveralls -coverprofile=provile.cov -service=travis-ci + - goveralls -race -v -show -service=travis-ci diff --git a/newznab/newznab.go b/newznab/newznab.go index 3073ac2..53f512c 100644 --- a/newznab/newznab.go +++ b/newznab/newznab.go @@ -10,9 +10,8 @@ import ( "strings" "time" - "fmt" - log "github.com/Sirupsen/logrus" + "github.com/pkg/errors" ) // Various constants for categories @@ -173,7 +172,6 @@ func (c Client) search(vals url.Values) ([]NZB, error) { func (c Client) process(vals url.Values, path string) ([]NZB, error) { var nzbs []NZB - log.Debug("newznab:Client:Search: searching") resp, err := c.getURL(c.buildURL(vals, path)) if err != nil { return nzbs, err @@ -181,12 +179,11 @@ func (c Client) process(vals url.Values, path string) ([]NZB, error) { var feed SearchResponse err = xml.Unmarshal(resp, &feed) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to unmarshal xml feed") } if feed.ErrorCode != 0 { - return nil, fmt.Errorf("newznab api error %d: %s", feed.ErrorCode, feed.ErrorDesc) + return nil, errors.Errorf("newznab api error %d: %s", feed.ErrorCode, feed.ErrorDesc) } - log.WithField("num", len(feed.Channel.NZBs)).Debug("newznab:Client:Search: found NZBs") for _, gotNZB := range feed.Channel.NZBs { nzb := NZB{ Title: gotNZB.Title, @@ -200,7 +197,7 @@ func (c Client) process(vals url.Values, path string) ([]NZB, error) { switch attr.Name { case "tvairdate": if parsedAirDate, err := parseDate(attr.Value); err != nil { - log.Errorf("newznab:Client:Search: failed to parse tvairdate: %v", err) + log.WithError(err).WithField("tvairdate", attr.Value).Debug("newznab:Client:Search: failed to parse tvairdate") } else { nzb.AirDate = parsedAirDate } @@ -259,7 +256,7 @@ func (c Client) process(vals url.Values, path string) ([]NZB, error) { nzb.CoverURL = attr.Value case "usenetdate": if parsedUsetnetDate, err := parseDate(attr.Value); err != nil { - log.Errorf("newznab:Client:Search: failed to parse usenetdate: %v", err) + log.WithError(err).WithField("usenetdate", attr.Value).Debug("failed to parse usenetdate") } else { nzb.UsenetDate = parsedUsetnetDate } @@ -280,7 +277,6 @@ func (c Client) process(vals url.Values, path string) ([]NZB, error) { // PopulateComments fills in the Comments for the given NZB func (c Client) PopulateComments(nzb *NZB) error { - log.Debug("newznab:Client:PopulateComments: getting comments") data, err := c.getURL(c.buildURL(url.Values{ "t": []string{"comments"}, "id": []string{nzb.ID}, @@ -292,7 +288,7 @@ func (c Client) PopulateComments(nzb *NZB) error { var resp commentResponse err = xml.Unmarshal(data, &resp) if err != nil { - return err + return errors.Wrap(err, "failed to unmarshal comments xml data") } for _, rawComment := range resp.Channel.Comments { @@ -301,10 +297,7 @@ func (c Client) PopulateComments(nzb *NZB) error { Content: rawComment.Description, } if parsedPubDate, err := time.Parse(time.RFC1123Z, rawComment.PubDate); err != nil { - log.WithFields(log.Fields{ - "pub_date": rawComment.PubDate, - "err": err, - }).Error("newznab:Client:PopulateComments: failed to parse date") + log.WithError(err).WithField("pubdate", rawComment.PubDate).Debug("failed to parse comment date") } else { comment.PubDate = parsedPubDate } @@ -314,7 +307,7 @@ func (c Client) PopulateComments(nzb *NZB) error { } // NZBDownloadURL returns a URL to download the NZB from -func (c Client) NZBDownloadURL(nzb NZB) string { +func (c Client) NZBDownloadURL(nzb NZB) (string, error) { return c.buildURL(url.Values{ "t": []string{"get"}, "id": []string{nzb.ID}, @@ -327,34 +320,32 @@ func (c Client) DownloadNZB(nzb NZB) ([]byte, error) { return c.getURL(c.NZBDownloadURL(nzb)) } -func (c Client) getURL(url string) ([]byte, error) { - log.WithField("url", url).Debug("newznab:Client:getURL: getting url") - res, err := c.client.Get(url) +func (c Client) getURL(url string, err error) ([]byte, error) { if err != nil { return nil, err } + res, err := c.client.Get(url) + if err != nil { + return nil, errors.Wrapf(err, "http request failed: %s", url) + } var data []byte data, err = ioutil.ReadAll(res.Body) res.Body.Close() if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to read response body") } - - log.WithField("num_bytes", len(data)).Debug("newznab:Client:getURL: retrieved") - return data, nil } -func (c Client) buildURL(vals url.Values, path string) string { +func (c Client) buildURL(vals url.Values, path string) (string, error) { parsedURL, err := url.Parse(c.apiBaseURL + path) if err != nil { - log.WithError(err).Error("failed to parse base API url") - return "" + return "", errors.Wrap(err, "failed to parse base API url") } parsedURL.RawQuery = vals.Encode() - return parsedURL.String() + return parsedURL.String(), nil } func parseDate(date string) (time.Time, error) { @@ -366,7 +357,7 @@ func parseDate(date string) (time.Time, error) { return parsedTime, nil } } - return parsedTime, fmt.Errorf("failed to parse date %s as one of %s", date, strings.Join(formats, ", ")) + return parsedTime, errors.Errorf("failed to parse date %s as one of %s", date, strings.Join(formats, ", ")) } const ( diff --git a/newznab/newznab_test.go b/newznab/newznab_test.go index 1d40b58..e14ebbb 100644 --- a/newznab/newznab_test.go +++ b/newznab/newznab_test.go @@ -94,7 +94,8 @@ func TestUsenetCrawlerClient(t *testing.T) { }) t.Run("download url", func(t *testing.T) { - url := client.NZBDownloadURL(results[0]) + url, err := client.NZBDownloadURL(results[0]) + require.NoError(t, err) require.NotEmpty(t, url, "expected a url") })