tests(gh): write some tests around HTTP handling

.github/workflows/ci.yml

@@ -42,7 +42,7 @@ jobs:
           go-version-file: 'go.mod'
 
       - name: Test
-        run: go test -v ./...
+        run: go test -v -cover ./... | column -t -s'	'
 
   lint:
     runs-on: ubuntu-latest
@@ -89,7 +89,7 @@ jobs:
         run: |
           go install golang.org/x/tools/cmd/deadcode@latest
 
-          deadcode ./... > "deadcode.txt"
+          deadcode -test ./... > "deadcode.txt"
           if [ -s "deadcode.txt" ]; then
             echo "Dead code found:"
             cat deadcode.txt

.gitignore

@@ -27,3 +27,5 @@ go.work
 /cache-test.json
 
 dist/
+coverage
+coverage.html

internal/api/api.go

@@ -3,15 +3,9 @@ package api
 import (
 	"io"
 	"net/http"
-
-	ghapi "github.com/cli/go-gh/v2/pkg/api"
 )
 
 type Caller interface {
 	Do(string, string, io.Reader, interface{}) error
 	Request(string, string, io.Reader) (*http.Response, error)
 }
-
-func NewGH() (Caller, error) {
-	return ghapi.DefaultRESTClient()
-}

internal/api/github/github.go

@@ -0,0 +1,10 @@
+package github
+
+import (
+	gh "github.com/cli/go-gh/v2/pkg/api"
+	"github.com/nobe4/gh-not/internal/api"
+)
+
+func New() (api.Caller, error) {
+	return gh.DefaultRESTClient()
+}

internal/api/mock/mock.go

@@ -0,0 +1,71 @@
+package mock
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+
+	"github.com/nobe4/gh-not/internal/api"
+)
+
+type Mock struct {
+	Calls []Call
+
+	index int
+}
+
+type Call struct {
+	Verb     string
+	Endpoint string
+	Data     any
+	Error    error
+	Response *http.Response
+}
+
+type MockError struct {
+	verb     string
+	endpoint string
+	message  string
+}
+
+func (e *MockError) Error() string {
+	return fmt.Sprintf("mock error: %s %s: %s", e.verb, e.endpoint, e.message)
+}
+
+func New(c []Call) api.Caller {
+	return &Mock{Calls: c}
+}
+
+func (m *Mock) call(verb, endpoint string) (Call, error) {
+	if m.index >= len(m.Calls) {
+		return Call{}, &MockError{verb, endpoint, "no more calls"}
+	}
+
+	call := m.Calls[m.index]
+	if (call.Verb != "" && call.Verb != verb) || (call.Endpoint != "" && call.Endpoint != endpoint) {
+		return Call{}, &MockError{verb, endpoint, "unexpected call"}
+	}
+
+	m.index++
+
+	return call, nil
+}
+
+func (m *Mock) Do(verb, endpoint string, body io.Reader, out interface{}) error {
+	call, err := m.call(verb, endpoint)
+	if err != nil {
+		return err
+	}
+
+	out = call.Data
+	return call.Error
+}
+
+func (m *Mock) Request(verb, endpoint string, body io.Reader) (*http.Response, error) {
+	call, err := m.call(verb, endpoint)
+	if err != nil {
+		return nil, err
+	}
+
+	return call.Response, call.Error
+}

internal/cmd/root.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/nobe4/gh-not/internal/api"
 	"github.com/nobe4/gh-not/internal/api/file"
+	"github.com/nobe4/gh-not/internal/api/github"
 	configPkg "github.com/nobe4/gh-not/internal/config"
 	managerPkg "github.com/nobe4/gh-not/internal/manager"
 	"github.com/spf13/cobra"
@@ -77,7 +78,7 @@ func setupGlobals(cmd *cobra.Command, args []string) error {
 	if notificationDumpPath != "" {
 		caller = file.New(notificationDumpPath)
 	} else {
-		caller, err = api.NewGH()
+		caller, err = github.New()
 		if err != nil {
 			slog.Error("Failed to create an API REST client", "err", err)
 			return err

internal/gh/enrichments.go

@@ -7,16 +7,18 @@ import (
 	"github.com/nobe4/gh-not/internal/notifications"
 )
 
+type Extra struct {
+	User    notifications.User `json:"user"`
+	State   string             `json:"state"`
+	HtmlUrl string             `json:"html_url"`
+}
+
 func (c *Client) enrichNotification(n *notifications.Notification) error {
 	if n.Meta.Done {
 		return nil
 	}
 
-	extra := struct {
-		User    notifications.User `json:"user"`
-		State   string             `json:"state"`
-		HtmlUrl string             `json:"html_url"`
-	}{}
+	extra := Extra{}
 	if err := c.API.Do(http.MethodGet, n.Subject.URL, nil, &extra); err != nil {
 		return err
 	}

internal/gh/errors.go

@@ -0,0 +1,12 @@
+package gh
+
+import "fmt"
+
+type RetryError struct {
+	verb     string
+	endpoint string
+}
+
+func (e RetryError) Error() string {
+	return fmt.Sprintf("retry exceeded for %s %s", e.verb, e.endpoint)
+}

internal/gh/errors_test.go

@@ -0,0 +1,16 @@
+package gh
+
+import "testing"
+
+func TestRetryError(t *testing.T) {
+	e := RetryError{
+		verb:     "verb",
+		endpoint: "endpoint",
+	}
+	want := "retry exceeded for verb endpoint"
+	got := e.Error()
+
+	if got != want {
+		t.Errorf("got %s, want %s", got, want)
+	}
+}

internal/gh/gh.go

@@ -4,7 +4,6 @@ package gh
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"log/slog"
 	"net/http"
@@ -21,6 +20,10 @@ const (
 	DefaultEndpoint = "https://api.github.com/notifications"
 )
 
+var (
+	linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`)
+)
+
 type Client struct {
 	API      api.Caller
 	cache    cache.ExpiringReadWriter
@@ -44,84 +47,97 @@ func NewClient(api api.Caller, cache cache.ExpiringReadWriter, config config.End
 	}
 }
 
-func isRetryable(err error) bool {
+func isRetryable(e error) bool {
 	var httpError *ghapi.HTTPError
-
-	if errors.As(err, &httpError) {
+	if errors.As(e, &httpError) {
 		switch httpError.StatusCode {
 		case 502, 504:
 			return true
 		}
 	}
 
+	if errors.Is(e, io.EOF) {
+		return true
+	}
+
 	return false
 }
 
-func (c *Client) retryRequest(verb, endpoint string, body io.Reader) (*http.Response, error) {
-	for i := c.maxRetry; i > 0; i-- {
-		response, err := c.API.Request(verb, endpoint, body)
-		if err != nil {
-			if isRetryable(err) {
-				slog.Warn("endpoint failed with retryable status", "endpoint", endpoint, "retry left", i)
-				continue
-			}
+func parse(r *http.Response) ([]*notifications.Notification, string, error) {
+	n := []*notifications.Notification{}
 
-			return nil, err
+	decoder := json.NewDecoder(r.Body)
+	if err := decoder.Decode(&n); err != nil {
+		return nil, "", err
+	}
+	defer r.Body.Close()
+
+	return n, nextPageLink(&r.Header), nil
+}
+
+func nextPageLink(h *http.Header) string {
+	for _, m := range linkRE.FindAllStringSubmatch(h.Get("Link"), -1) {
+		if len(m) > 2 && m[2] == "next" {
+			return m[1]
 		}
+	}
+	return ""
+}
 
-		return response, nil
+func (c *Client) request(verb, endpoint string, body io.Reader) ([]*notifications.Notification, string, error) {
+	response, err := c.API.Request(verb, endpoint, body)
+	if err != nil {
+		return nil, "", err
 	}
 
-	return nil, fmt.Errorf("retry exceeded for %s", endpoint)
+	return parse(response)
 }
 
-// inspired by https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/example_gh_test.go#L96-L134
-func (c *Client) paginateNotifications() (notifications.Notifications, error) {
-	var list notifications.Notifications
+func (c *Client) retry(verb, endpoint string, body io.Reader) ([]*notifications.Notification, string, error) {
+	for i := c.maxRetry; i >= 0; i-- {
+		notifications, next, err := c.request(verb, endpoint, body)
+		if err == nil {
+			return notifications, next, nil
+		}
 
-	var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`)
-	findNextPage := func(response *http.Response) string {
-		for _, m := range linkRE.FindAllStringSubmatch(response.Header.Get("Link"), -1) {
-			if len(m) > 2 && m[2] == "next" {
-				return m[1]
-			}
+		if isRetryable(err) {
+			slog.Warn("endpoint failed with retryable error", "endpoint", endpoint, "retry left", i)
+			continue
 		}
-		return ""
+
+		return nil, "", err
 	}
 
+	return nil, "", RetryError{verb, endpoint}
+}
+
+// inspired by https://github.com/cli/go-gh/blob/25db6b99518c88e03f71dbe9e58397c4cfb62caf/example_gh_test.go#L96-L134
+func (c *Client) paginate() (notifications.Notifications, error) {
+	var list notifications.Notifications
+	var pageList []*notifications.Notification
+	var err error
+
 	pageLeft := c.maxPage
 	endpoint := c.endpoint
 
-	for endpoint != "" && pageLeft > 0 {
+	for endpoint != "" && pageLeft >= 0 {
 		slog.Info("API REST request", "endpoint", endpoint, "page_left", pageLeft)
 
-		response, err := c.retryRequest(http.MethodGet, endpoint, nil)
-		if err != nil {
-			return nil, err
-		}
-
-		pageList := notifications.Notifications{}
-		decoder := json.NewDecoder(response.Body)
-		err = decoder.Decode(&pageList)
+		pageList, endpoint, err = c.retry(http.MethodGet, endpoint, nil)
 		if err != nil {
 			return nil, err
 		}
 
 		list = append(list, pageList...)
 
-		if err := response.Body.Close(); err != nil {
-			return nil, err
-		}
-
-		endpoint = findNextPage(response)
 		pageLeft--
 	}
 
 	return list, nil
 }
 
-func (c *Client) pullNotificationFromApi() (notifications.Notifications, error) {
-	list, err := c.paginateNotifications()
+func (c *Client) Notifications() (notifications.Notifications, error) {
+	list, err := c.paginate()
 	if err != nil {
 		return nil, err
 	}
@@ -136,20 +152,3 @@ func (c *Client) pullNotificationFromApi() (notifications.Notifications, error)
 
 	return list, nil
 }
-
-func (c *Client) Notifications() (notifications.Notifications, error) {
-	allNotifications := notifications.Notifications{}
-
-	pulledNotifications, err := c.pullNotificationFromApi()
-	if err != nil {
-		return nil, err
-	}
-
-	allNotifications = append(allNotifications, pulledNotifications...)
-
-	if err := c.cache.Write(allNotifications); err != nil {
-		slog.Error("Error while writing the cache: %#v", err)
-	}
-
-	return allNotifications.Uniq(), nil
-}