Skip to content

Commit e1a8e59

Browse files
Added unit tests for fetcher, checker and matchers packages (#185)
* Tests for matchers package #173 Added unit tests for the 3 functions in matchers.go * Tests for checker package #173 Changed checker to accept interface of type Fetcher for easier mocking. Added tests for all the possible errors that could happen in Check * Tests for fetcher package #173 Refactored fetcher to use fetcher.sendRequest instead of hclient.Do for easier testing. Added tests for fetch function for all possible errors * style(test): Simplified return statements in fetcher and checker tests Fixed lint issues associated with return statements Co-authored-by: Preslav Mihaylov <pmihaylov95@gmail.com>
1 parent d3b91b6 commit e1a8e59

File tree

5 files changed

+327
-6
lines changed

5 files changed

+327
-6
lines changed

checker/checker.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,21 @@ import (
55
"fmt"
66

77
checkererrors "github.com/preslavmihaylov/todocheck/checker/errors"
8-
"github.com/preslavmihaylov/todocheck/fetcher"
98
"github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
109
"github.com/preslavmihaylov/todocheck/matchers"
1110
)
1211

12+
type Fetcher interface {
13+
Fetch(taskID string) (taskstatus.TaskStatus, error)
14+
}
15+
1316
// Checker for todo lines
1417
type Checker struct {
15-
statusFetcher *fetcher.Fetcher
18+
statusFetcher Fetcher
1619
}
1720

1821
// New checker
19-
func New(statusFetcher *fetcher.Fetcher) *Checker {
22+
func New(statusFetcher Fetcher) *Checker {
2023
return &Checker{statusFetcher}
2124
}
2225

checker/checker_test.go

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package checker
2+
3+
import (
4+
"errors"
5+
"reflect"
6+
"testing"
7+
8+
checkerrors "github.com/preslavmihaylov/todocheck/checker/errors"
9+
"github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
10+
)
11+
12+
func TestCheck(t *testing.T) {
13+
fetcher := mockFetcher{}
14+
checker := New(&fetcher)
15+
matcher := mockMatcher{}
16+
17+
testLines := []string{}
18+
testLineCnt := 0
19+
20+
testData := []struct {
21+
comment, filename string
22+
todoErr *checkerrors.TODO
23+
err error
24+
}{
25+
{"NotMatch", "", nil, nil},
26+
{"NotValid", "test.go", checkerrors.MalformedTODOErr("test.go", testLines, testLineCnt), nil},
27+
{"FailedFetch", "", nil, errors.New("")},
28+
{"ClosedIssue", "test.go", checkerrors.IssueClosedErr("test.go", testLines, testLineCnt, "ClosedIssue"), nil},
29+
{"NonExistentIssue", "test.go", checkerrors.IssueNonExistentErr("test.go", testLines, testLineCnt, "NonExistentIssue"), nil},
30+
{"Valid", "", nil, nil},
31+
}
32+
for _, tt := range testData {
33+
t.Run(tt.comment, func(t *testing.T) {
34+
35+
todoErr, err := checker.Check(matcher, tt.comment, tt.filename, testLines, testLineCnt)
36+
if !reflect.DeepEqual(todoErr, tt.todoErr) {
37+
t.Errorf("Expected toddErr to be %v, got %v", tt.todoErr, todoErr)
38+
}
39+
if (err == nil) != (tt.err == nil) { // Don't care about the error string
40+
t.Errorf("Expected err to be %v, got %v", tt.err, err)
41+
}
42+
})
43+
}
44+
45+
t.Run("NilMatcher", func(t *testing.T) {
46+
todoErr, err := checker.Check(nil, "", "", testLines, testLineCnt)
47+
if todoErr != nil {
48+
t.Errorf("Expected toddErr to be nil, got %v", todoErr)
49+
}
50+
if err == nil { // Don't care about the error string
51+
t.Errorf("Expected err to be not nil")
52+
}
53+
})
54+
t.Run("InvalidExtract", func(t *testing.T) {
55+
defer func() {
56+
if r := recover(); r == nil {
57+
t.Logf("Recovered in %v", r)
58+
}
59+
}()
60+
_, err := checker.Check(matcher, "InvalidExtract", "", testLines, testLineCnt)
61+
if err != nil {
62+
t.Errorf("Expected err to be nil, got %v", err)
63+
}
64+
t.Errorf("Expected code to panic")
65+
})
66+
}
67+
68+
type mockMatcher struct {
69+
}
70+
71+
func (m mockMatcher) IsMatch(expr string) bool {
72+
return expr != "NotMatch"
73+
}
74+
func (m mockMatcher) IsValid(expr string) bool {
75+
return expr != "NotValid"
76+
}
77+
func (m mockMatcher) ExtractIssueRef(expr string) (string, error) {
78+
if expr == "InvalidExtract" {
79+
return expr, errors.New("Invalid todo")
80+
}
81+
return expr, nil
82+
}
83+
84+
type mockFetcher struct {
85+
}
86+
87+
func (f *mockFetcher) Fetch(taskID string) (taskstatus.TaskStatus, error) {
88+
if taskID == "FailedFetch" {
89+
return 0, errors.New("FailedFetch")
90+
}
91+
if taskID == "ClosedIssue" {
92+
return 2, nil
93+
}
94+
if taskID == "NonExistentIssue" {
95+
return 3, nil
96+
}
97+
return 0, nil
98+
}

fetcher/fetcher.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ import (
1313
// Fetcher for task statuses by contacting task management web apps' rest api
1414
type Fetcher struct {
1515
issuetracker.IssueTracker
16+
sendRequest func(req *http.Request) (*http.Response, error)
1617
}
1718

1819
// NewFetcher instance
1920
func NewFetcher(issueTracker issuetracker.IssueTracker) *Fetcher {
20-
return &Fetcher{issueTracker}
21+
httpClient := &http.Client{}
22+
return &Fetcher{issueTracker, httpClient.Do}
2123
}
2224

2325
// Fetch a task's status based on task ID
@@ -32,8 +34,7 @@ func (f *Fetcher) Fetch(taskID string) (taskstatus.TaskStatus, error) {
3234
return taskstatus.None, fmt.Errorf("couldn't instrument authentication middleware: %w", err)
3335
}
3436

35-
hclient := &http.Client{}
36-
resp, err := hclient.Do(req)
37+
resp, err := f.sendRequest(req)
3738
if err != nil {
3839
return taskstatus.None, fmt.Errorf("couldn't execute GET request: %w", err)
3940
}

fetcher/fetcher_test.go

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package fetcher
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"errors"
7+
"io"
8+
"net/http"
9+
"testing"
10+
11+
"github.com/preslavmihaylov/todocheck/issuetracker"
12+
"github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
13+
)
14+
15+
var errTest = errors.New("")
16+
17+
func TestFetch(t *testing.T) {
18+
fetcher := NewFetcher(mockIssueTracker{})
19+
testJSON, err := json.Marshal(mockTask{})
20+
if err != nil {
21+
t.Fatalf("Test json is bad")
22+
}
23+
24+
testData := []struct {
25+
Task string
26+
Client mockClient
27+
Status int
28+
Err error
29+
}{
30+
{
31+
Task: "GoodFetch",
32+
Client: mockClient{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: nil},
33+
Status: 1,
34+
Err: nil,
35+
},
36+
{
37+
Task: "BadURL",
38+
Client: mockClient{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: nil},
39+
Status: 0,
40+
Err: errTest,
41+
},
42+
{
43+
Task: "MiddlewareFailure",
44+
Client: mockClient{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: nil},
45+
Status: 0,
46+
Err: errTest,
47+
},
48+
{
49+
Task: "FailedSendingRequest",
50+
Client: mockClient{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: errTest},
51+
Status: 0,
52+
Err: errTest,
53+
},
54+
{
55+
Task: "BadReader",
56+
Client: mockClient{StatusCode: 200, Body: errReader(0), Err: nil},
57+
Status: 0,
58+
Err: errTest,
59+
},
60+
{
61+
Task: "ResponseStatusNotFound",
62+
Client: mockClient{StatusCode: 404, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: nil},
63+
Status: 3,
64+
Err: nil,
65+
},
66+
{
67+
Task: "ResponseBadStatus",
68+
Client: mockClient{StatusCode: 405, Body: io.NopCloser(bytes.NewReader(testJSON)), Err: nil},
69+
Status: 0,
70+
Err: errTest,
71+
},
72+
{
73+
Task: "BadJson",
74+
Client: mockClient{StatusCode: 200, Body: io.NopCloser(bytes.NewReader([]byte{})), Err: nil},
75+
Status: 0,
76+
Err: errTest,
77+
},
78+
}
79+
for _, tt := range testData {
80+
t.Run(tt.Task, func(t *testing.T) {
81+
fetcher.sendRequest = tt.Client.sendRequest
82+
taskStatus, err := fetcher.Fetch(tt.Task)
83+
if taskStatus != taskstatus.TaskStatus(tt.Status) {
84+
t.Errorf("Task status is %v, expected %v", taskStatus, taskstatus.TaskStatus(tt.Status))
85+
}
86+
if (err == nil) != (tt.Err == nil) { // Doesn't care about error message or type
87+
t.Errorf("Fetch error is %v, expected %v", err, tt.Err)
88+
}
89+
})
90+
}
91+
92+
}
93+
94+
// Mocking Task
95+
type mockTask struct {
96+
Status string
97+
}
98+
99+
func (t mockTask) GetStatus() (taskstatus.TaskStatus, error) {
100+
return taskstatus.Open, nil
101+
}
102+
103+
// Mocking IssueTracker
104+
type mockIssueTracker struct {
105+
}
106+
107+
func (it mockIssueTracker) TaskModel() issuetracker.Task {
108+
return &mockTask{}
109+
}
110+
111+
func (it mockIssueTracker) IssueURLFor(taskID string) string {
112+
if taskID == "BadURL" {
113+
return string(byte(' ') - 1) // This causes http.NewRequest to fail
114+
}
115+
return taskID
116+
}
117+
118+
func (it mockIssueTracker) Exists() bool { // Never called
119+
return false
120+
}
121+
122+
func (it mockIssueTracker) InstrumentMiddleware(r *http.Request) error {
123+
if r.URL.Path == "MiddlewareFailure" { // The taskID is set as URL path, so we're using that as our fail trigger
124+
return errTest
125+
}
126+
return nil
127+
}
128+
129+
func (it mockIssueTracker) TokenAcquisitionInstructions() string { // Never called
130+
return ""
131+
}
132+
133+
// Mocking sendRequest
134+
type mockClient struct {
135+
StatusCode int
136+
Body io.ReadCloser
137+
Err error
138+
}
139+
140+
func (c mockClient) sendRequest(req *http.Request) (*http.Response, error) {
141+
return &http.Response{
142+
StatusCode: c.StatusCode,
143+
Body: c.Body,
144+
}, c.Err
145+
}
146+
147+
type errReader int
148+
149+
func (r errReader) Read(body []byte) (int, error) {
150+
return 0, errTest
151+
}
152+
153+
func (r errReader) Close() error {
154+
return nil
155+
}

matchers/matchers_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package matchers
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/preslavmihaylov/todocheck/matchers/state"
8+
)
9+
10+
func TestTodoMatcherForFile(t *testing.T) {
11+
testTodo := []string{"// TODO"}
12+
13+
for extension, factory := range supportedMatchers {
14+
t.Run(extension, func(t *testing.T) {
15+
matcher := TodoMatcherForFile("test"+extension, testTodo)
16+
want := factory.newTodoMatcher(testTodo)
17+
if matcher != want {
18+
t.Errorf("got %v want %v", matcher, want)
19+
}
20+
})
21+
}
22+
23+
t.Run("Unsupported extension", func(t *testing.T) {
24+
matcher := TodoMatcherForFile("test.md", testTodo)
25+
if matcher != nil {
26+
t.Errorf("Expected nil matcher")
27+
}
28+
})
29+
}
30+
31+
func TestCommentMatcherForFile(t *testing.T) {
32+
var testCommentCallback state.CommentCallback
33+
34+
for extension, factory := range supportedMatchers {
35+
t.Run(extension, func(t *testing.T) {
36+
matcher := CommentMatcherForFile("test"+extension, testCommentCallback)
37+
want := factory.newCommentsMatcher(testCommentCallback)
38+
if !reflect.DeepEqual(matcher, want) {
39+
t.Errorf("got %v want %v", matcher, want)
40+
}
41+
})
42+
}
43+
44+
t.Run("Unsupported extension", func(t *testing.T) {
45+
matcher := CommentMatcherForFile("test.md", testCommentCallback)
46+
if matcher != nil {
47+
t.Errorf("Expected nil matcher")
48+
}
49+
})
50+
}
51+
52+
func TestSupportedFileExtensions(t *testing.T) {
53+
extensions := SupportedFileExtensions()
54+
55+
for _, ext := range extensions {
56+
if _, ok := supportedMatchers[ext]; !ok {
57+
t.Errorf("Extension %v is not in supported extensions", ext)
58+
}
59+
}
60+
61+
if len(extensions) != len(supportedMatchers) {
62+
t.Errorf("Some extensions from supported extensions are missing")
63+
}
64+
}

0 commit comments

Comments
 (0)