Improve tests

FerretDB · AlekSi · Jul 26, 2023 · Jun 20, 2023 · Jun 21, 2023 · Jun 21, 2023
commit eaf50a95fccc4df43fb1efdaeb9a016c63b2d50e
@@ -68,7 +68,7 @@ type QueryResult struct {
 
 // Query executes a query against the collection.
 //
-// If the collection does not exist it returns empty iterator.
+// If database or collection does not exist it returns empty iterator.
 //
 // The passed context should be used for canceling the initial query.
 // It also can be used to close the returned iterator and free underlying resources,
@@ -108,6 +108,8 @@ func (cc *collectionContract) Insert(ctx context.Context, params *InsertParams)
 
 // UpdateParams represents the parameters of Collection.Update method.
 type UpdateParams struct {
+	// TODO https://github.com/FerretDB/FerretDB/issues/3079
+	// that should be types.DocumentIterator
 	Docs *types.Array
 }
 
@@ -117,6 +119,8 @@ type UpdateResult struct {
 }
 
 // Update updates documents in collection.
+//
+// Database or collection may not exist; that's not an error.
 func (cc *collectionContract) Update(ctx context.Context, params *UpdateParams) (*UpdateResult, error) {
 	defer observability.FuncCall(ctx)()
 
@@ -138,7 +142,7 @@ type DeleteResult struct {
 
 // Delete deletes documents in collection.
 //
-// If requested database or collection does not exist it returns 0 deleted documents.
+// Database or collection may not exist; that's not an error.
 func (cc *collectionContract) Delete(ctx context.Context, params *DeleteParams) (*DeleteResult, error) {
 	defer observability.FuncCall(ctx)()
 

@@ -93,7 +93,7 @@ type CollectionInfo struct {
 
 // ListCollections returns information about collections in the database.
 //
-// Database doesn't have to exist; that's not an error.
+// Database may not exist; that's not an error.
 func (dbc *databaseContract) ListCollections(ctx context.Context, params *ListCollectionsParams) (*ListCollectionsResult, error) {
 	defer observability.FuncCall(ctx)()
 

@@ -40,8 +40,8 @@ const (
 
 // Error represents a backend error returned by all Backend, Database and Collection methods.
 type Error struct {
-	// this internal error can't be accessed by the caller;
-	// it may be nil
+	// This internal error can't be accessed by the caller; it exists only for debugging.
+	// It may be nil.
 	err error
 
 	code ErrorCode

@@ -18,7 +18,6 @@ import (
 	"context"
 
 	"go.uber.org/zap"
-	_ "modernc.org/sqlite"
 
 	"github.com/FerretDB/FerretDB/internal/backends"
 	"github.com/FerretDB/FerretDB/internal/backends/sqlite/metadata"

diff --git a/internal/backends/sqlite/metadata/pool/pool.go b/internal/backends/sqlite/metadata/pool/pool.go
@@ -52,9 +52,9 @@ type Pool struct {
 	token *resource.Token
 }
 
-// New creates a pool for SQLite databases in the given directory.
+// New creates a pool for SQLite databases in the directory specified by SQLite URI.
 func New(u string, l *zap.Logger) (*Pool, error) {
-	uri, err := validateURI(u)
+	uri, err := parseURI(u)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse SQLite URI %q: %s", u, err)
 	}
@@ -91,50 +91,6 @@ func New(u string, l *zap.Logger) (*Pool, error) {
 	return p, nil
 }
 
-// validateURI checks given URI value and returns parsed URL.
-// URI should contain 'file' scheme and point to an existing directory.
-// Path should end with '/'. Authority should be empty or absent.
-//
-// Returned URL contains path in both Path and Opaque to make String() method work correctly.
-func validateURI(u string) (*url.URL, error) {
-	uri, err := url.Parse(u)
-	if err != nil {
-		return nil, err
-	}
-
-	if uri.Scheme != "file" {
-		return nil, fmt.Errorf(`expected "file:" schema, got %q`, uri.Scheme)
-	}
-
-	if uri.User != nil {
-		return nil, fmt.Errorf(`expected empty user info, got %q`, uri.User)
-	}
-
-	if uri.Host != "" {
-		return nil, fmt.Errorf(`expected empty host, got %q`, uri.Host)
-	}
-
-	if uri.Path == "" && uri.Opaque != "" {
-		uri.Path = uri.Opaque
-	}
-	uri.Opaque = uri.Path
-
-	if !strings.HasSuffix(uri.Path, "/") {
-		return nil, fmt.Errorf(`expected path ending with "/", got %q`, uri.Path)
-	}
-
-	fi, err := os.Stat(uri.Path)
-	if err != nil {
-		return nil, fmt.Errorf(`%q should be an existing directory, got %s`, uri.Path, err)
-	}
-
-	if !fi.IsDir() {
-		return nil, fmt.Errorf(`%q should be an existing directory`, uri.Path)
-	}
-
-	return uri, nil
-}
-
 // databaseName returns database name for given database file path.
 func (p *Pool) databaseName(databaseFile string) string {
 	return strings.TrimSuffix(filepath.Base(databaseFile), filenameExtension)

diff --git a/internal/backends/sqlite/metadata/pool/pool_test.go b/internal/backends/sqlite/metadata/pool/pool_test.go
@@ -15,11 +15,8 @@
 package pool
 
 import (
-	"net/url"
-	"os"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/FerretDB/FerretDB/internal/util/testutil"
@@ -58,127 +55,3 @@ func TestCreateDrop(t *testing.T) {
 	db = p.GetExisting(ctx, t.Name())
 	require.Nil(t, db)
 }
-
-func TestNewBackend(t *testing.T) {
-	t.Parallel()
-
-	err := os.MkdirAll("tmp/dir", 0o777)
-	require.NoError(t, err)
-
-	_, err = os.Create("tmp/file")
-	require.NoError(t, err)
-
-	t.Cleanup(func() {
-		err := os.RemoveAll("tmp")
-		require.NoError(t, err)
-	})
-
-	testCases := map[string]struct {
-		value string
-		uri   *url.URL
-		err   string
-	}{
-		"LocalDirectory": {
-			value: "file:./",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./",
-				Path:   "./",
-			},
-		},
-		"LocalSubDirectory": {
-			value: "file:./tmp/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./tmp/",
-				Path:   "./tmp/",
-			},
-		},
-		"LocalSubSubDirectory": {
-			value: "file:./tmp/dir/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./tmp/dir/",
-				Path:   "./tmp/dir/",
-			},
-		},
-		"LocalDirectoryWithParameters": {
-			value: "file:./tmp/?mode=ro",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "./tmp/",
-				Path:     "./tmp/",
-				RawQuery: "mode=ro",
-			},
-		},
-		"AbsoluteDirectory": {
-			value: "file:/tmp/",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "/tmp/",
-				Path:     "/tmp/",
-				OmitHost: true,
-			},
-		},
-		"WithEmptyAuthority": {
-			value: "file:///tmp/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "/tmp/",
-				Path:   "/tmp/",
-			},
-		},
-		"WithEmptyAuthorityAndQuery": {
-			value: "file:///tmp/?mode=ro",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "/tmp/",
-				Path:     "/tmp/",
-				RawQuery: "mode=ro",
-			},
-		},
-		"HostIsNotEmpty": {
-			value: "file://localhost/./tmp/?mode=ro",
-			err:   `expected empty host, got "localhost"`,
-		},
-		"UserIsNotEmpty": {
-			value: "file://user:pass@./tmp/?mode=ro",
-			err:   `expected empty user info, got "user:pass"`,
-		},
-		"NoDirectory": {
-			value: "file:./nodir/",
-			err:   `"./nodir/" should be an existing directory, got stat ./nodir/: no such file or directory`,
-		},
-		"PathIsNotEndsWithSlash": {
-			value: "file:./tmp/file",
-			err:   `expected path ending with "/", got "./tmp/file"`,
-		},
-		"FileInsteadOfDirectory": {
-			value: "file:./tmp/file/",
-			err:   `"./tmp/file/" should be an existing directory, got stat ./tmp/file/: not a directory`,
-		},
-		"MalformedURI": {
-			value: ":./tmp/",
-			err:   `parse ":./tmp/": missing protocol scheme`,
-		},
-		"NoScheme": {
-			value: "./tmp/",
-			err:   `expected "file:" schema, got ""`,
-		},
-	}
-	for name, tc := range testCases {
-		name, tc := name, tc
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			u, err := validateURI(tc.value)
-			if tc.err != "" {
-				assert.EqualError(t, err, tc.err)
-				return
-			}
-
-			require.NoError(t, err)
-			assert.Equal(t, u, tc.uri)
-		})
-	}
-}
diff --git a/internal/backends/sqlite/metadata/pool/uri.go b/internal/backends/sqlite/metadata/pool/uri.go
@@ -0,0 +1,70 @@
+// Copyright 2021 FerretDB Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pool
+
+import (
+	"fmt"
+	"net/url"
+	"os"
+	"strings"
+)
+
+// parseURI checks given SQLite URI and returns a parsed form.
+//
+// URI should contain 'file' scheme and point to an existing directory.
+// Path should end with '/'. Authority should be empty or absent.
+//
+// Returned URL contains path in both Path and Opaque to make String() method work correctly.
+// Callers should use Path.
+func parseURI(u string) (*url.URL, error) {
+	uri, err := url.Parse(u)
+	if err != nil {
+		return nil, err
+	}
+
+	if uri.Scheme != "file" {
+		return nil, fmt.Errorf(`expected "file:" schema, got %q`, uri.Scheme)
+	}
+
+	if uri.User != nil {
+		return nil, fmt.Errorf(`expected empty user info, got %q`, uri.User)
+	}
+
+	if uri.Host != "" {
+		return nil, fmt.Errorf(`expected empty host, got %q`, uri.Host)
+	}
+
+	if uri.Path == "" {
+		uri.Path = uri.Opaque
+	}
+	uri.Opaque = uri.Path
+	uri.RawPath = ""
+	uri.OmitHost = true
+
+	if !strings.HasSuffix(uri.Path, "/") {
+		return nil, fmt.Errorf(`expected path ending with "/", got %q`, uri.Path)
+	}
+
+	fi, err := os.Stat(uri.Path)
+	if err != nil {
+		return nil, fmt.Errorf(`%q should be an existing directory, got %s`, uri.Path, err)
+	}
+
+	if !fi.IsDir() {
+		return nil, fmt.Errorf(`%q should be an existing directory`, uri.Path)
+	}
+
+	return uri, nil
+}