From e06e8c4e25cc3176ac67d818a5cf98909f2cd821 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Fri, 10 Mar 2023 22:37:25 -0500 Subject: [PATCH 1/5] make tests fully table driven and increase coverage by a lot --- internal/api/api_test.go | 303 ++++++++++++++++++++++++--------------- 1 file changed, 188 insertions(+), 115 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 5f87f59..6f67767 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -88,10 +88,9 @@ func TestAPI(t *testing.T) { db.Create(&internal.User{Username: tester.username, HashedPassword: string(hashedPassword)}) - tester.TestMakeAccount(t) - tester.TestLogin(t) - tester.TestLogout(t) - tester.TestUpload(t) + tester.TestOpenRoutes(t) + tester.TestProtectedRoutes(t) + tester.TestRateLimit(t) tester.CleanUp(db) } @@ -115,119 +114,203 @@ func Init(url string) accountTest { return test } -func (a *accountTest) TestMakeAccount(t *testing.T) { - var target response - - client := http.Client{} - - form := url.Values{} - - form.Add("username", a.username) - - form.Add("password", a.password) - - if req, err := http.NewRequest(http.MethodPost, a.url+`api/create`, strings.NewReader(form.Encode())); err == nil { - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - - resp, err := client.Do(req) - if err != nil { - assert.NotNil(t, err) - } - - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } - - assert.Equal(t, http.StatusCreated, target.Status) - } -} - -func (a *accountTest) TestLogin(t *testing.T) { - var target response - - client := http.Client{} - - form := url.Values{} - - form.Add("username", a.username) - - form.Add("password", a.password) - - if req, err := http.NewRequest(http.MethodPost, a.url+`/api/login`, strings.NewReader(form.Encode())); err == nil { - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - - resp, err := client.Do(req) - if err != nil { - assert.NotNil(t, err) - } - - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } - - assert.Equal(t, http.StatusOK, resp.StatusCode) - } -} - -func (a *accountTest) TestLogout(t *testing.T) { - var target response - - client := http.Client{} - - form := url.Values{} - - form.Add("username", a.username) - - if req, err := http.NewRequest(http.MethodPost, a.url+`/api/logout`, strings.NewReader(form.Encode())); err == nil { - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - - resp, err := client.Do(req) - if err != nil { - assert.NotNil(t, err) - } - - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } - - assert.Equal(t, http.StatusOK, resp.StatusCode) - } -} - type expectedValues struct { response int } -func (a *accountTest) TestUpload(t *testing.T) { +func (a *accountTest) TestOpenRoutes(t *testing.T) { + testCases := []struct { + name string + route string + token string + password string + username string + expected expectedValues + }{ + { + name: "Should fail to login with bad username", + route: "login", + username: "a.username", + password: a.password, + expected: expectedValues{ + response: http.StatusUnauthorized, + }, + }, + { + name: "Should fail to login with bad password", + route: "login", + username: a.username, + password: "a.password", + expected: expectedValues{ + response: http.StatusUnauthorized, + }, + }, + { + name: "Should fail to login with bad password or username", + route: "login", + username: "apple", + password: "a.password", + expected: expectedValues{ + response: http.StatusUnauthorized, + }, + }, + { + name: "Should fail to login with empty form", + route: "login", + expected: expectedValues{ + response: http.StatusInternalServerError, + }, + }, + { + name: "Should login successfully", + route: "login", + username: a.username, + password: a.password, + expected: expectedValues{ + response: http.StatusOK, + }, + }, + { + name: "Should fail to create account with empty form", + route: "create", + expected: expectedValues{ + response: http.StatusInternalServerError, + }, + }, + { + name: "Should create account with empty username", + route: "create", + password: "asdf", + expected: expectedValues{ + response: http.StatusCreated, + }, + }, + { + name: "Should fail to create account with empty password", + route: "create", + expected: expectedValues{ + response: http.StatusInternalServerError, + }, + }, + { + name: "Should log out successfully", + route: "logout", + username: a.username, + expected: expectedValues{ + response: http.StatusOK, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var target response + + client := http.Client{} + + form := url.Values{} + + form.Add("username", tc.username) + + form.Add("password", tc.password) + + if req, err := http.NewRequest(http.MethodPost, a.url+`/api/`+tc.route, strings.NewReader(form.Encode())); err == nil { + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + + resp, err := client.Do(req) + if err != nil { + assert.NotNil(t, err) + } + + respBody, _ := io.ReadAll(resp.Body) + + err = json.Unmarshal(respBody, &target) + if err != nil { + assert.NotNil(t, err) + } + + assert.Equal(t, tc.expected.response, resp.StatusCode) + } + }) + } +} + +func (a *accountTest) TestRateLimit(t *testing.T) { + testCases := []struct { + name string + route string + password string + username string + expected expectedValues + }{ + { + name: "Should rate limit open routes", + route: "login", + expected: expectedValues{ + response: http.StatusTooManyRequests, + }, + }, + { + name: "Should rate limit private routes", + route: "upload", + expected: expectedValues{ + response: http.StatusTooManyRequests, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var target response + + client := http.Client{} + + form := url.Values{} + + if req, err := http.NewRequest(http.MethodPost, a.url+`/api/`+tc.route, strings.NewReader(form.Encode())); err == nil { + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + + for i := 0; i < 100; i++ { + resp, err := client.Do(req) + if err != nil { + assert.NotNil(t, err) + } + + respBody, _ := io.ReadAll(resp.Body) + + err = json.Unmarshal(respBody, &target) + if err != nil { + assert.NotNil(t, err) + } + if resp.StatusCode == tc.expected.response { + assert.Equal(t, tc.expected.response, resp.StatusCode) + } + } + } + }) + } +} + +func (a *accountTest) TestProtectedRoutes(t *testing.T) { testCases := []struct { name string - file string contentType string + route string fileContents []byte expected expectedValues }{ { name: "Should fail to upload an empty file", - file: "badzonefile", contentType: "audio/aac", + route: "upload", expected: expectedValues{ response: http.StatusInternalServerError, }, }, { name: "Should upload a valid file", - file: "zonefile", contentType: "multipart/form-data", fileContents: []byte{}, + route: "upload", expected: expectedValues{ response: http.StatusCreated, }, @@ -244,18 +327,12 @@ func (a *accountTest) TestUpload(t *testing.T) { w = multipart.NewWriter(buf) ) - switch tc.name { - case "Should fail to upload an empty file": - f, err = os.CreateTemp(".", tc.file) - if err != nil { - assert.NotNil(t, err) - } - default: - f, err = os.CreateTemp(".", "zonefile") - if err != nil { - assert.NotNil(t, err) - } + f, err = os.CreateTemp(".", "zonefile") + if err != nil { + assert.NotNil(t, err) + } + if tc.name == "Should upload a valid file" { if err = os.WriteFile(f.Name(), zonebytes, 0o600); err != nil { assert.NotNil(t, err) } @@ -263,8 +340,6 @@ func (a *accountTest) TestUpload(t *testing.T) { defer os.Remove(f.Name()) //nolint: errcheck - client := http.Client{} - part, err := w.CreateFormFile("file", filepath.Base(f.Name())) if err != nil { assert.NotNil(t, err) @@ -284,12 +359,11 @@ func (a *accountTest) TestUpload(t *testing.T) { if err != nil { assert.NotNil(t, err) } - - if req, err := http.NewRequest(http.MethodPost, a.url+`/api/upload`, buf); err == nil { - switch tc.name { - case "Should fail to upload an empty file": + client := http.Client{} + if req, err := http.NewRequest(http.MethodPost, a.url+`/api/`+tc.route, buf); err == nil { + if tc.name == "Should fail to upload an empty file" { req.Header.Add("Content-Type", tc.contentType) - default: + } else { req.Header.Add("Content-Type", w.FormDataContentType()) } @@ -306,7 +380,6 @@ func (a *accountTest) TestUpload(t *testing.T) { if err != nil { assert.NotNil(t, err) } - assert.Equal(t, tc.expected.response, resp.StatusCode) } }) From b5302bdc8c84e0bd32534a08f9c3e772ee09c329 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 11 Mar 2023 06:54:09 -0500 Subject: [PATCH 2/5] remove unused code and file. --- internal/api/api_test.go | 52 ++++----------------------------------- internal/logger/logger.go | 1 - 2 files changed, 5 insertions(+), 48 deletions(-) delete mode 100644 internal/logger/logger.go diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 6f67767..ebee398 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -2,9 +2,7 @@ package api import ( "bytes" - "encoding/json" "fmt" - "io" "mime/multipart" "net/http" "net/http/httptest" @@ -24,12 +22,6 @@ import ( "gorm.io/gorm" ) -type response struct { - Username string `json:"username"` - Message string `json:"message"` - Status int `json:"status"` -} - type accountTest struct { username string password string @@ -52,7 +44,7 @@ func makeTestToken(username string) (tokenString string, err error) { func TestAPI(t *testing.T) { config, err := internal.ReadConfig() if err != nil { - panic(err) + assert.NotNil(t, err) } pomme := chi.NewRouter() @@ -204,8 +196,6 @@ func (a *accountTest) TestOpenRoutes(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - var target response - client := http.Client{} form := url.Values{} @@ -222,13 +212,6 @@ func (a *accountTest) TestOpenRoutes(t *testing.T) { assert.NotNil(t, err) } - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } - assert.Equal(t, tc.expected.response, resp.StatusCode) } }) @@ -260,8 +243,6 @@ func (a *accountTest) TestRateLimit(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - var target response - client := http.Client{} form := url.Values{} @@ -275,12 +256,6 @@ func (a *accountTest) TestRateLimit(t *testing.T) { assert.NotNil(t, err) } - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } if resp.StatusCode == tc.expected.response { assert.Equal(t, tc.expected.response, resp.StatusCode) } @@ -320,11 +295,10 @@ func (a *accountTest) TestProtectedRoutes(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { var ( - f *os.File - target response - err error - buf = new(bytes.Buffer) - w = multipart.NewWriter(buf) + f *os.File + err error + buf = new(bytes.Buffer) + w = multipart.NewWriter(buf) ) f, err = os.CreateTemp(".", "zonefile") @@ -373,13 +347,6 @@ func (a *accountTest) TestProtectedRoutes(t *testing.T) { if err != nil { assert.NotNil(t, err) } - - respBody, _ := io.ReadAll(resp.Body) - - err = json.Unmarshal(respBody, &target) - if err != nil { - assert.NotNil(t, err) - } assert.Equal(t, tc.expected.response, resp.StatusCode) } }) @@ -387,15 +354,6 @@ func (a *accountTest) TestProtectedRoutes(t *testing.T) { } func (a *accountTest) CleanUp(db *gorm.DB) { - var ( - user internal.User - req internal.ZoneRequest - ) - - db.Where("username = ?", a.username).Delete(&user) - - db.Where("user = ?", a.username).Delete(&req) - if err := os.Remove("pomme-test.sqlite"); err != nil { l := newResponder(Response[any]{ Message: "unable to clean up test DB", diff --git a/internal/logger/logger.go b/internal/logger/logger.go deleted file mode 100644 index 90c66f6..0000000 --- a/internal/logger/logger.go +++ /dev/null @@ -1 +0,0 @@ -package logger From 6ecf051982ded905b2833da4e5d9a910a68fa613 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 11 Mar 2023 09:08:13 -0500 Subject: [PATCH 3/5] add more tests --- internal/configuration.go | 6 ++-- internal/configuration_test.go | 51 ++++++++++++++++++++++++++++++++++ internal/db/db_test.go | 34 +++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 internal/configuration_test.go create mode 100644 internal/db/db_test.go diff --git a/internal/configuration.go b/internal/configuration.go index 0c10101..73d6dab 100644 --- a/internal/configuration.go +++ b/internal/configuration.go @@ -22,7 +22,7 @@ func ReadConfig() (*Config, error) { if data, err = os.ReadFile(filepath.Clean(defaultConfigPath)); err == nil { if err = yaml.Unmarshal(data, &config); err != nil { - return &Config{}, fmt.Errorf("unable to unmarshal config file: %w", err) + return nil, fmt.Errorf("unable to unmarshal config file: %w", err) } return &config, nil @@ -30,11 +30,11 @@ func ReadConfig() (*Config, error) { if data, err = os.ReadFile(filepath.Clean("./config.yaml")); err == nil { if err = yaml.Unmarshal(data, &config); err != nil { - return &Config{}, fmt.Errorf("unable to unmarshal config file: %w", err) + return nil, fmt.Errorf("unable to unmarshal config file: %w", err) } return &config, nil } - return &Config{}, fmt.Errorf("unable to read config file: %w", err) + return nil, fmt.Errorf("unable to read config file: %w", err) } diff --git a/internal/configuration_test.go b/internal/configuration_test.go new file mode 100644 index 0000000..14e0d94 --- /dev/null +++ b/internal/configuration_test.go @@ -0,0 +1,51 @@ +package internal + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConfig(t *testing.T) { + testCases := []struct { + name string + fileContents string + }{ + { + name: "Should fail to read config", + }, + { + name: "Should read config file successfully", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + if tc.name == "Should read config file successfully" { + f, err := os.Create("config.yaml") + if err != nil { + assert.NotNil(t, err) + } + defer os.Remove(f.Name()) + } else { + err := os.Remove("config.yaml") + if err != nil { + assert.NotNil(t, err) + } + } + + config, err := ReadConfig() + if err != nil { + assert.NotNil(t, err) + } + + switch tc.name { + case "Should read config file successfully": + assert.NotNil(t, config) + default: + assert.Nil(t, config) + } + }) + } +} diff --git a/internal/db/db_test.go b/internal/db/db_test.go new file mode 100644 index 0000000..4b452d1 --- /dev/null +++ b/internal/db/db_test.go @@ -0,0 +1,34 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDB(t *testing.T) { + testCases := []struct { + name string + dbMode string + dbName string + }{ + { + name: "Should fail to open db", + dbMode: "nothing", + dbName: "", + }, + { + name: "Should open test db", + dbMode: "test", + dbName: "", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err, ok := InitDb(tc.dbName, tc.dbMode) + if err != nil && !ok { + assert.NotNil(t, err) + } + }) + } +} From ac42ad6196e4f2bb8ba42f93f4eb943518c83b62 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 11 Mar 2023 09:14:10 -0500 Subject: [PATCH 4/5] lint, and change test paths in CI --- Makefile | 2 +- internal/configuration_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ecfe683..a8fcc12 100644 --- a/Makefile +++ b/Makefile @@ -48,4 +48,4 @@ test-frontend: pnpm -F frontend "test:unit" test-backend: - go test -v -cover ./internal/api + go test -v -cover ./internal/... diff --git a/internal/configuration_test.go b/internal/configuration_test.go index 14e0d94..9616f80 100644 --- a/internal/configuration_test.go +++ b/internal/configuration_test.go @@ -21,13 +21,12 @@ func TestConfig(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - if tc.name == "Should read config file successfully" { f, err := os.Create("config.yaml") if err != nil { assert.NotNil(t, err) } - defer os.Remove(f.Name()) + defer os.Remove(f.Name()) //nolint: errcheck } else { err := os.Remove("config.yaml") if err != nil { From 60b7d675508827db06631cbbf59618de2aaee949 Mon Sep 17 00:00:00 2001 From: grumbulon Date: Sat, 18 Mar 2023 14:38:58 -0400 Subject: [PATCH 5/5] test nonsense --- internal/api/api_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index ebee398..f8da10c 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -67,16 +67,11 @@ func TestAPI(t *testing.T) { tester := Init(ts.URL) // test mode mode = "test" - db, err, ok := db.InitDb(config.DB, mode) - - if err != nil && !ok { - assert.NotNil(t, err) - } + db, err, _ := db.InitDb(config.DB, mode) + assert.Nil(t, err) hashedPassword, err := bcrypt.GenerateFromPassword([]byte(tester.password), bcrypt.DefaultCost) - if err != nil { - assert.NotNil(t, err) - } + assert.Nil(t, err) db.Create(&internal.User{Username: tester.username, HashedPassword: string(hashedPassword)})