From 08516fde596fd2e1d3a7a454b6ddee4b7b9d3336 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Tue, 18 Jun 2019 22:19:57 +0200 Subject: [PATCH] gitbase: fix close repository test Also fix some repositories left open. Signed-off-by: Javi Fontan --- blobs.go | 7 +- blobs_test.go | 12 +- commit_blobs_test.go | 52 ++++--- commit_files_test.go | 12 +- commit_trees_test.go | 12 +- commits_test.go | 24 +-- common_test.go | 358 ++++++++++++++++++++----------------------- files_test.go | 12 +- partition.go | 3 + ref_commits_test.go | 12 +- references_test.go | 24 +-- remotes_test.go | 16 +- tree_entries_test.go | 12 +- 13 files changed, 265 insertions(+), 291 deletions(-) diff --git a/blobs.go b/blobs.go index d0dab016d..5d3098bc0 100644 --- a/blobs.go +++ b/blobs.go @@ -347,12 +347,7 @@ func newBlobsKeyValueIter( return nil, err } - r, err := pool.GetRepo(repo.ID()) - if err != nil { - return nil, err - } - - idx, err := newRepositoryIndex(r) + idx, err := newRepositoryIndex(repo) if err != nil { return nil, err } diff --git a/blobs_test.go b/blobs_test.go index 13afab6d4..39c5a3f81 100644 --- a/blobs_test.go +++ b/blobs_test.go @@ -238,10 +238,10 @@ func TestBlobsIndex(t *testing.T) { ) } -// func TestBlobsIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(blobsTable)) -// } +func TestBlobsIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(blobsTable)) +} -// func TestBlobsIterClosed(t *testing.T) { -// testTableIterClosed(t, new(blobsTable)) -// } +func TestBlobsIterClosed(t *testing.T) { + testTableIterClosed(t, new(blobsTable)) +} diff --git a/commit_blobs_test.go b/commit_blobs_test.go index ce83680bd..17f3c95f1 100644 --- a/commit_blobs_test.go +++ b/commit_blobs_test.go @@ -1,10 +1,12 @@ package gitbase import ( + "io" "testing" "github.com/src-d/go-mysql-server/sql" "github.com/src-d/go-mysql-server/sql/expression" + "github.com/src-d/go-mysql-server/sql/plan" "github.com/stretchr/testify/require" "gopkg.in/src-d/go-git.v4/plumbing" ) @@ -203,28 +205,28 @@ func TestCommitBlobsRowKeyMapper(t *testing.T) { require.Equal(row, row2) } -// func TestCommitBlobsIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(commitBlobsTable)) -// } - -// // This one is not using testTableIterClosed as it takes too much time -// // to go through all the rows. Here we limit it to the first 100. -// func TestCommitBlobsIterClosed(t *testing.T) { -// require := require.New(t) -// ctx, closed := setupSivaCloseRepos(t, "_testdata") - -// table := new(commitBlobsTable) -// iter, err := plan.NewResolvedTable(table).RowIter(ctx) -// require.NoError(err) - -// for i := 0; i < 100; i++ { -// _, err = iter.Next() -// if err != nil { -// require.Equal(io.EOF, err) -// break -// } -// } - -// iter.Close() -// require.True(closed.Check()) -// } +func TestCommitBlobsIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(commitBlobsTable)) +} + +// This one is not using testTableIterClosed as it takes too much time +// to go through all the rows. Here we limit it to the first 100. +func TestCommitBlobsIterClosed(t *testing.T) { + require := require.New(t) + ctx, closed := setupSivaCloseRepos(t, "_testdata") + + table := new(commitBlobsTable) + iter, err := plan.NewResolvedTable(table).RowIter(ctx) + require.NoError(err) + + for i := 0; i < 100; i++ { + _, err = iter.Next() + if err != nil { + require.Equal(io.EOF, err) + break + } + } + + iter.Close() + require.True(closed.Check()) +} diff --git a/commit_files_test.go b/commit_files_test.go index 7e01be316..9c879ddda 100644 --- a/commit_files_test.go +++ b/commit_files_test.go @@ -121,13 +121,13 @@ func TestEncodeCommitFileIndexKey(t *testing.T) { require.Equal(k, k2) } -// func TestCommitFilesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(commitFilesTable)) -// } +func TestCommitFilesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(commitFilesTable)) +} -// func TestCommitFilesIterClosed(t *testing.T) { -// testTableIterClosed(t, new(commitFilesTable)) -// } +func TestCommitFilesIterClosed(t *testing.T) { + testTableIterClosed(t, new(commitFilesTable)) +} func TestPartitionRowsWithIndex(t *testing.T) { require := require.New(t) diff --git a/commit_trees_test.go b/commit_trees_test.go index 64ef01e57..26832eee0 100644 --- a/commit_trees_test.go +++ b/commit_trees_test.go @@ -169,10 +169,10 @@ func TestCommitTreesRowKeyMapper(t *testing.T) { require.Equal(row, row2) } -// func TestCommitTreesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(commitTreesTable)) -// } +func TestCommitTreesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(commitTreesTable)) +} -// func TestCommitTreesIterClosed(t *testing.T) { -// testTableIterClosed(t, new(commitTreesTable)) -// } +func TestCommitTreesIterClosed(t *testing.T) { + testTableIterClosed(t, new(commitTreesTable)) +} diff --git a/commits_test.go b/commits_test.go index ab7db77e0..06fa93649 100644 --- a/commits_test.go +++ b/commits_test.go @@ -297,15 +297,15 @@ func TestCommitsIndex(t *testing.T) { ) } -// func TestCommitsIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(commitsTable)) -// } - -// func TestCommitsIterClosed(t *testing.T) { -// testTableIterClosed(t, new(commitsTable)) -// } - -// func TestCommitsIterators(t *testing.T) { -// // columns names just for debugging -// testTableIterators(t, new(commitsTable), []string{"commit_hash", "commit_author_email"}) -// } +func TestCommitsIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(commitsTable)) +} + +func TestCommitsIterClosed(t *testing.T) { + testTableIterClosed(t, new(commitsTable)) +} + +func TestCommitsIterators(t *testing.T) { + // columns names just for debugging + testTableIterators(t, new(commitsTable), []string{"commit_hash", "commit_author_email"}) +} diff --git a/common_test.go b/common_test.go index 8f9091928..0b94c729d 100644 --- a/common_test.go +++ b/common_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "github.com/src-d/go-borges" @@ -78,221 +79,194 @@ func tableToRows(ctx *sql.Context, t sql.Table) ([]sql.Row, error) { return sql.NodeToRows(ctx, plan.NewResolvedTable(t)) } -// /* +/* -// The following code adds utilities to test that siva files are properly closed. -// Instead of using normal setup you can use setupSivaCloseRepos, it returns -// a context with a pool with all the sivas in "_testdata" directory. It also -// tracks all siva filesystems opened. Its closed state can be checked with -// closedSiva.Check(). +The following code adds utilities to test that siva files are properly closed. +Instead of using normal setup you can use setupSivaCloseRepos, it returns +a context with a pool with all the sivas in "_testdata" directory. It also +tracks all siva filesystems opened. Its closed state can be checked with +closedLib.Check(). -// */ +*/ -// type closedSiva struct { -// closed []bool -// m sync.Mutex -// } +type closedRepository struct { + borges.Repository + closed bool +} + +func (c *closedRepository) Close() error { + c.closed = true + return c.Repository.Close() +} + +type closedLibrary struct { + *multiLibrary + repos []*closedRepository + m sync.Mutex +} + +func (c *closedLibrary) trackRepo(r borges.Repository) *closedRepository { + c.m.Lock() + defer c.m.Unlock() + + closed := &closedRepository{Repository: r} + c.repos = append(c.repos, closed) + + return closed +} + +func (c *closedLibrary) Check() bool { + for _, r := range c.repos { + if !r.closed { + return false + } + } + return true +} + +func (c *closedLibrary) Get( + r borges.RepositoryID, + m borges.Mode, +) (borges.Repository, error) { + repo, err := c.multiLibrary.Get(r, m) + if err != nil { + return nil, err + } + + return c.trackRepo(repo), nil +} + +func (c *closedLibrary) Repositories(m borges.Mode) (borges.RepositoryIterator, error) { + iter, err := c.multiLibrary.Repositories(m) + if err != nil { + return nil, err + } + + return &closedIter{ + RepositoryIterator: iter, + c: c, + }, nil +} + +type closedIter struct { + borges.RepositoryIterator + c *closedLibrary +} + +func (i *closedIter) Next() (borges.Repository, error) { + repo, err := i.RepositoryIterator.Next() + if err != nil { + return nil, err + } + + return i.c.trackRepo(repo), nil +} + +// setupSivaCloseRepos creates a pool with siva files that can be checked +// if they've been closed. +func setupSivaCloseRepos(t *testing.T, dir string) (*sql.Context, *closedLibrary) { + require := require.New(t) -// func (c *closedSiva) NewFS(path string) (billy.Filesystem, error) { -// c.m.Lock() -// defer c.m.Unlock() + t.Helper() -// localfs := osfs.New(filepath.Dir(path)) + lib, err := newMultiLibrary() + require.NoError(err) -// tmpDir, err := ioutil.TempDir(os.TempDir(), "gitbase-siva") -// if err != nil { -// return nil, err -// } + closedLib := &closedLibrary{multiLibrary: lib} + pool := NewRepositoryPool(cache.DefaultMaxSize, closedLib) -// tmpfs := osfs.New(tmpDir) + cwd, err := os.Getwd() + require.NoError(err) + cwdFS := osfs.New(cwd) -// fs, err := sivafs.NewFilesystem(localfs, filepath.Base(path), tmpfs) -// if err != nil { -// return nil, err -// } + filepath.Walk(dir, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } -// pos := len(c.closed) -// c.closed = append(c.closed, false) + if IsSivaFile(path) { + require.NoError(lib.AddSiva(path, cwdFS)) + } -// fun := func() { -// c.m.Lock() -// defer c.m.Unlock() -// c.closed[pos] = true -// } + return nil + }, + ) -// return &closedSivaFilesystem{fs, fun}, nil -// } + session := NewSession(pool, WithSkipGitErrors(true)) + ctx := sql.NewContext(context.TODO(), sql.WithSession(session)) -// func (c *closedSiva) Check() bool { -// for _, f := range c.closed { -// if !f { -// return false -// } -// } + return ctx, closedLib +} -// return true -// } +func testTableIndexIterClosed(t *testing.T, table sql.IndexableTable) { + t.Helper() -// type closedSivaFilesystem struct { -// sivafs.SivaFS -// closeFunc func() -// } + require := require.New(t) + ctx, closed := setupSivaCloseRepos(t, "_testdata") -// func (c *closedSivaFilesystem) Sync() error { -// if c.closeFunc != nil { -// c.closeFunc() -// } + iter, err := table.IndexKeyValues(ctx, nil) + require.NoError(err) -// return c.SivaFS.Sync() -// } + for { + _, i, err := iter.Next() + if err != nil { + require.Equal(io.EOF, err) + break + } -// var _ *Repository = new(closedSivaRepository) + i.Close() + } -// type closedSivaRepository struct { -// *git.Repository -// path string -// siva *closedSiva -// cache cache.Object -// } + iter.Close() + require.True(closed.Check()) +} -// func (c *closedSivaRepository) ID() string { -// return c.path -// } +func testTableIterators(t *testing.T, table sql.IndexableTable, columns []string) { + t.Helper() -// // func (c *closedSivaRepository) Repo() (*Repository, error) { -// // fs, err := c.FS() -// // if err != nil { -// // return nil, err -// // } + require := require.New(t) + ctx, closed := setupSivaCloseRepos(t, "_testdata") -// // s := fs.(*closedSivaFilesystem) -// // closeFunc := func() { s.Sync() } + rows, _ := tableToRows(ctx, table) + expected := len(rows) -// // sto := filesystem.NewStorageWithOptions(fs, c.Cache(), gitStorerOptions) -// // repo, err := git.Open(sto, nil) -// // if err != nil { -// // return nil, err - -// // } + iter, err := table.IndexKeyValues(ctx, columns) + require.NoError(err) + actual := 0 + for { + _, i, err := iter.Next() + if err != nil { + require.Equal(io.EOF, err) + break + } + for { + _, _, err := i.Next() + if err != nil { + require.Equal(io.EOF, err) + break + } + actual++ + } + + i.Close() + } + iter.Close() + require.True(closed.Check()) -// // return NewRepository(c.path, repo, closeFunc), nil -// // } - -// func (c *closedSivaRepository) FS() (billy.Filesystem, error) { -// return c.siva.NewFS(c.path) -// } - -// func (c *closedSivaRepository) Path() string { -// return c.path -// } - -// func (c *closedSivaRepository) Cache() cache.Object { -// if c.cache == nil { -// c.cache = cache.NewObjectLRUDefault() -// } - -// return c.cache -// } - -// // setupSivaCloseRepos creates a pool with siva files that can be checked -// // if they've been closed. -// func setupSivaCloseRepos(t *testing.T, dir string) (*sql.Context, *closedSiva) { -// require := require.New(t) - -// t.Helper() - -// lib, err := newMultiLibrary() -// require.NoError(err) - -// cs := new(closedSiva) -// pool := NewRepositoryPool(cache.DefaultMaxSize, lib) - -// filepath.Walk(dir, -// func(path string, info os.FileInfo, err error) error { -// if strings.HasSuffix(path, ".siva") { -// repo := &closedSivaRepository{path: path, siva: cs} -// err := pool.Add(repo) -// require.NoError(err) -// } - -// return nil -// }, -// ) - -// session := NewSession(pool, WithSkipGitErrors(true)) -// ctx := sql.NewContext(context.TODO(), sql.WithSession(session)) - -// return ctx, cs -// } - -// func testTableIndexIterClosed(t *testing.T, table sql.IndexableTable) { -// t.Helper() - -// require := require.New(t) -// ctx, closed := setupSivaCloseRepos(t, "_testdata") - -// iter, err := table.IndexKeyValues(ctx, nil) -// require.NoError(err) - -// for { -// _, i, err := iter.Next() -// if err != nil { -// require.Equal(io.EOF, err) -// break -// } - -// i.Close() -// } - -// iter.Close() -// require.True(closed.Check()) -// } - -// func testTableIterators(t *testing.T, table sql.IndexableTable, columns []string) { -// t.Helper() - -// require := require.New(t) -// ctx, closed := setupSivaCloseRepos(t, "_testdata") - -// rows, _ := tableToRows(ctx, table) -// expected := len(rows) - -// iter, err := table.IndexKeyValues(ctx, columns) -// require.NoError(err) -// actual := 0 -// for { -// _, i, err := iter.Next() -// if err != nil { -// require.Equal(io.EOF, err) -// break -// } -// for { -// _, _, err := i.Next() -// if err != nil { -// require.Equal(io.EOF, err) -// break -// } -// actual++ -// } - -// i.Close() -// } -// iter.Close() -// require.True(closed.Check()) - -// require.EqualValues(expected, actual) -// } - -// func testTableIterClosed(t *testing.T, table sql.IndexableTable) { -// t.Helper() - -// require := require.New(t) -// ctx, closed := setupSivaCloseRepos(t, "_testdata") -// _, err := tableToRows(ctx, table) -// require.NoError(err) - -// require.True(closed.Check()) -// } + require.EqualValues(expected, actual) +} + +func testTableIterClosed(t *testing.T, table sql.IndexableTable) { + t.Helper() + + require := require.New(t) + ctx, closed := setupSivaCloseRepos(t, "_testdata") + _, err := tableToRows(ctx, table) + require.NoError(err) + + require.True(closed.Check()) +} func poolFromCtx(t *testing.T, ctx *sql.Context) *RepositoryPool { t.Helper() diff --git a/files_test.go b/files_test.go index bc6d845f7..998238e3e 100644 --- a/files_test.go +++ b/files_test.go @@ -842,10 +842,10 @@ func TestEncodeFileIndexKey(t *testing.T) { require.Equal(k, k3) } -// func TestFilesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(filesTable)) -// } +func TestFilesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(filesTable)) +} -// func TestFilesIterClosed(t *testing.T) { -// testTableIterClosed(t, new(filesTable)) -// } +func TestFilesIterClosed(t *testing.T) { + testTableIterClosed(t, new(filesTable)) +} diff --git a/partition.go b/partition.go index 5d742f4da..d3e0577d2 100644 --- a/partition.go +++ b/partition.go @@ -85,6 +85,9 @@ func (i *repositoryPartitionIter) Next() (sql.Partition, error) { } } + // TODO: RepositoryPartition should hold the repository so we don't + // get it twice. + defer r.Close() return RepositoryPartition(r.ID().String()), nil } diff --git a/ref_commits_test.go b/ref_commits_test.go index e501b38fc..6568c3c41 100644 --- a/ref_commits_test.go +++ b/ref_commits_test.go @@ -183,10 +183,10 @@ func TestRefCommitsRowKeyMapper(t *testing.T) { require.Equal(row, row2) } -// func TestRefCommitsIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(refCommitsTable)) -// } +func TestRefCommitsIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(refCommitsTable)) +} -// func TestRefCommitsIterClosed(t *testing.T) { -// testTableIterClosed(t, new(refCommitsTable)) -// } +func TestRefCommitsIterClosed(t *testing.T) { + testTableIterClosed(t, new(refCommitsTable)) +} diff --git a/references_test.go b/references_test.go index 80857119b..fe99a4d06 100644 --- a/references_test.go +++ b/references_test.go @@ -133,15 +133,15 @@ func TestRefRowKeyMapper(t *testing.T) { require.Equal(row, row2) } -// func TestReferencesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(referencesTable)) -// } - -// func TestReferencesIterClosed(t *testing.T) { -// testTableIterClosed(t, new(referencesTable)) -// } - -// func TestReferencesIterators(t *testing.T) { -// // columns names just for debugging -// testTableIterators(t, new(referencesTable), []string{"ref_name", "commit_hash"}) -// } +func TestReferencesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(referencesTable)) +} + +func TestReferencesIterClosed(t *testing.T) { + testTableIterClosed(t, new(referencesTable)) +} + +func TestReferencesIterators(t *testing.T) { + // columns names just for debugging + testTableIterators(t, new(referencesTable), []string{"ref_name", "commit_hash"}) +} diff --git a/remotes_test.go b/remotes_test.go index cc7e0ba4e..e9a76ca4b 100644 --- a/remotes_test.go +++ b/remotes_test.go @@ -156,11 +156,11 @@ func TestEncodeRemoteIndexKey(t *testing.T) { require.Equal(k, k2) } -// func TestRemotesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(remotesTable)) -// } - -// func TestRemotesIterators(t *testing.T) { -// // columns names just for debugging -// testTableIterators(t, new(remotesTable), []string{"remote_name", "remote_push_url"}) -// } +func TestRemotesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(remotesTable)) +} + +func TestRemotesIterators(t *testing.T) { + // columns names just for debugging + testTableIterators(t, new(remotesTable), []string{"remote_name", "remote_push_url"}) +} diff --git a/tree_entries_test.go b/tree_entries_test.go index 17903e672..75e8cb7f4 100644 --- a/tree_entries_test.go +++ b/tree_entries_test.go @@ -684,10 +684,10 @@ func TestEncodeTreeEntriesIndexKey(t *testing.T) { require.Equal(k, k3) } -// func TestTreeEntriesIndexIterClosed(t *testing.T) { -// testTableIndexIterClosed(t, new(treeEntriesTable)) -// } +func TestTreeEntriesIndexIterClosed(t *testing.T) { + testTableIndexIterClosed(t, new(treeEntriesTable)) +} -// func TestTreeEntriesIterClosed(t *testing.T) { -// testTableIterClosed(t, new(treeEntriesTable)) -// } +func TestTreeEntriesIterClosed(t *testing.T) { + testTableIterClosed(t, new(treeEntriesTable)) +}