Test/database sqlmock

CHANGELOG.md

@@ -49,6 +49,7 @@ Emojis for the following are chosen based on [gitmoji](https://gitmoji.dev/).
 - The `update_data.sh` download command was fixed to specify the correct dump snapshot location ([#65](https://github.com/scribe-org/Scribe-Server/pull/65)).
 - Trusted proxies are now explicitly configured for security.
 - `/packs` requests with `..` in the path are rejected so they can't show whether directories outside of the packs directory exist, `/packs/sqlite/list` returns `[]` instead of `null` when there are no SQLite files, and SQLite download file names are quoted ([#91](https://github.com/scribe-org/Scribe-Server/issues/91)).
+- Database queries for languages, data types, table schemas and table data return an error instead of partial results if reading rows fails partway through ([#92](https://github.com/scribe-org/Scribe-Server/issues/92)).
 - German data contract fixes for missing `displayValue` fields on declensions, and fully indexed conjugations/declensions.
 
 ### 📝 Documentation
@@ -77,6 +78,7 @@ Emojis for the following are chosen based on [gitmoji](https://gitmoji.dev/).
 - Unit tests were added for the language statistics retrieval endpoint handler ([#86](https://github.com/scribe-org/Scribe-Server/issues/86)).
 - Unit tests were added for the `/packs` static file and SQLite download handler, including directory traversal attempts, and for the Swagger documentation routes ([#91](https://github.com/scribe-org/Scribe-Server/issues/91)).
 - Unit tests were added for API route registration, the home page, server startup and configuration loading ([#90](https://github.com/scribe-org/Scribe-Server/issues/90)).
+- Unit tests were added for the database package using [go-sqlmock](https://github.com/DATA-DOG/go-sqlmock) ([#92](https://github.com/scribe-org/Scribe-Server/issues/92)).
 - Unit tests were added for the language data retrieval endpoint handler ([#87](https://github.com/scribe-org/Scribe-Server/issues/87)).
 
 ### ♻️ Code Refactoring

Makefile

@@ -8,8 +8,7 @@ ENV ?= dev
 GIN_MODE ?= debug
 
 # Minimum total test coverage (%) required by CI.
-# Note: Raise this as tests are added so that coverage can only go up.
-MIN_COVERAGE ?=45
+MIN_COVERAGE ?=70
 
 # Clean any build artifacts.
 clean:

database/connection.go

@@ -17,6 +17,8 @@ import (
 // It should be initialized via InitDatabase before use.
 var DB *sql.DB
 
+var openDatabase = sql.Open
+
 // InitDatabase initializes the database connection.
 func InitDatabase() error {
 	dsn := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s?charset=utf8mb4&parseTime=True&loc=Local",
@@ -28,7 +30,7 @@ func InitDatabase() error {
 	)
 
 	var err error
-	DB, err = sql.Open("mysql", dsn)
+	DB, err = openDatabase("mysql", dsn)
 	if err != nil {
 		return fmt.Errorf("error opening database: %w", err)
 	}

database/connection_test.go

@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package database
+
+import (
+	"database/sql"
+	"errors"
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// openedDatabase records how InitDatabase opened the database.
+type openedDatabase struct {
+	driver string
+	dsn    string
+}
+
+// setupInitDatabase sets the database config and replaces opening the database with the given connection or error.
+// Tests using it must not call t.Parallel() as it changes the DB, openDatabase and Viper globals.
+func setupInitDatabase(t *testing.T, db *sql.DB, openErr error) *openedDatabase {
+	t.Helper()
+
+	viper.Reset()
+	t.Cleanup(viper.Reset)
+	viper.Set("database.user", "scribe")
+	viper.Set("database.password", "secret")
+	viper.Set("database.host", "localhost")
+	viper.Set("database.port", "3306")
+	viper.Set("database.name", testDatabaseName)
+
+	originalDB, originalOpen := DB, openDatabase
+	t.Cleanup(func() { DB, openDatabase = originalDB, originalOpen })
+
+	opened := &openedDatabase{}
+	openDatabase = func(driver, dsn string) (*sql.DB, error) {
+		opened.driver, opened.dsn = driver, dsn
+		return db, openErr
+	}
+
+	return opened
+}
+
+// newPingMockDB returns a sqlmock connection that checks pings.
+func newPingMockDB(t *testing.T) (*sql.DB, sqlmock.Sqlmock) {
+	t.Helper()
+
+	db, mock, err := sqlmock.New(sqlmock.MonitorPingsOption(true))
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		assert.NoError(t, mock.ExpectationsWereMet())
+		_ = db.Close()
+	})
+
+	return db, mock
+}
+
+func TestInitDatabase(t *testing.T) {
+	t.Run("connects with the configured database", func(t *testing.T) {
+		db, mock := newPingMockDB(t)
+		mock.ExpectPing()
+		opened := setupInitDatabase(t, db, nil)
+
+		require.NoError(t, InitDatabase())
+
+		assert.Equal(t, "mysql", opened.driver)
+		assert.Equal(t, "scribe:secret@tcp(localhost:3306)/scribe_test?charset=utf8mb4&parseTime=True&loc=Local", opened.dsn)
+		assert.Same(t, db, DB)
+	})
+
+	t.Run("error when the database can't be opened", func(t *testing.T) {
+		setupInitDatabase(t, nil, errors.New("unknown driver"))
+
+		err := InitDatabase()
+
+		assert.ErrorContains(t, err, "error opening database: unknown driver")
+	})
+
+	t.Run("error when the database can't be reached", func(t *testing.T) {
+		db, mock := newPingMockDB(t)
+		mock.ExpectPing().WillReturnError(errors.New("connection refused"))
+		setupInitDatabase(t, db, nil)
+
+		err := InitDatabase()
+
+		assert.ErrorContains(t, err, "error pinging database: connection refused")
+	})
+}

database/helpers_test.go

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package database
+
+import (
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// testDatabaseName is the database name that queries are expected to use in tests.
+const testDatabaseName = "scribe_test"
+
+// newMockDB replaces the database connection with sqlmock and sets the database name for a test.
+// Tests using it must not call t.Parallel() as it changes the DB and Viper globals.
+func newMockDB(t *testing.T) sqlmock.Sqlmock {
+	t.Helper()
+
+	viper.Reset()
+	viper.Set("database.name", testDatabaseName)
+	t.Cleanup(viper.Reset)
+
+	db, mock, err := sqlmock.New()
+	require.NoError(t, err)
+
+	original := DB
+	DB = db
+	t.Cleanup(func() {
+		// Every expected query must have been sent.
+		assert.NoError(t, mock.ExpectationsWereMet())
+
+		DB = original
+		// Note: Closing isn't an expected call for sqlmock, so its error isn't checked.
+		_ = db.Close()
+	})
+
+	return mock
+}

database/language.go

@@ -42,6 +42,11 @@ func GetAvailableLanguages() ([]string, error) {
 		languages = append(languages, strings.ToLower(lang))
 	}
 
+	// check to not return partial results.
+	if err := rows.Err(); err != nil {
+		return nil, fmt.Errorf("error reading languages: %w", err)
+	}
+
 	return languages, nil
 }
 
@@ -86,6 +91,10 @@ func GetLanguageDataTypes(lang string) ([]string, error) {
 		}
 	}
 
+	if err := rows.Err(); err != nil {
+		return nil, fmt.Errorf("error reading data types: %w", err)
+	}
+
 	return dataTypes, nil
 }