From bbf288a8088f0ab271e235da1105eead2302ebe8 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 10 Dec 2014 10:51:07 -0800 Subject: [PATCH 1/5] Adds another error test case for reading nonexistent files --- storagedriver/testsuites/testsuites.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index 3aa8642c2..abf95855b 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -396,9 +396,14 @@ func (suite *DriverSuite) TestContinueStreamAppend(c *check.C) { // fails. func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { filename := randomPath(32) + _, err := suite.StorageDriver.ReadStream(filename, 0) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + + _, err = suite.StorageDriver.ReadStream(filename, 64) + c.Assert(err, check.NotNil) + c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) } // TestList checks the returned list of keys after populating a directory tree. From 14a072cd5fcc2edeb1823ef3793a53583c382ac7 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 10 Dec 2014 10:53:51 -0800 Subject: [PATCH 2/5] Improves storagedriver tests for Move command semantics Ensures that Move will properly overwrite the file at the destination location. Also checks that Move of a nonexistent source file will NOT delete the file at the destination. --- storagedriver/testsuites/testsuites.go | 44 ++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index abf95855b..bbdc6a1c4 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -466,14 +466,54 @@ func (suite *DriverSuite) TestMove(c *check.C) { c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) } -// TestMoveNonexistent checks that moving a nonexistent key fails +// TestMoveOverwrite checks that a moved object no longer exists at the source +// path and overwrites the contents at the destination. +func (suite *DriverSuite) TestMoveOverwrite(c *check.C) { + sourcePath := randomPath(32) + destPath := randomPath(32) + sourceContents := randomContents(32) + destContents := randomContents(64) + + defer suite.StorageDriver.Delete(firstPart(sourcePath)) + defer suite.StorageDriver.Delete(firstPart(destPath)) + + err := suite.StorageDriver.PutContent(sourcePath, sourceContents) + c.Assert(err, check.IsNil) + + err = suite.StorageDriver.PutContent(destPath, destContents) + c.Assert(err, check.IsNil) + + err = suite.StorageDriver.Move(sourcePath, destPath) + c.Assert(err, check.IsNil) + + received, err := suite.StorageDriver.GetContent(destPath) + c.Assert(err, check.IsNil) + c.Assert(received, check.DeepEquals, sourceContents) + + _, err = suite.StorageDriver.GetContent(sourcePath) + c.Assert(err, check.NotNil) + c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) +} + +// TestMoveNonexistent checks that moving a nonexistent key fails and does not +// delete the data at the destination path. func (suite *DriverSuite) TestMoveNonexistent(c *check.C) { + contents := randomContents(32) sourcePath := randomPath(32) destPath := randomPath(32) - err := suite.StorageDriver.Move(sourcePath, destPath) + defer suite.StorageDriver.Delete(firstPart(destPath)) + + err := suite.StorageDriver.PutContent(destPath, contents) + c.Assert(err, check.IsNil) + + err = suite.StorageDriver.Move(sourcePath, destPath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + + received, err := suite.StorageDriver.GetContent(destPath) + c.Assert(err, check.IsNil) + c.Assert(received, check.DeepEquals, contents) } // TestDelete checks that the delete operation removes data from the storage From cb25cc65bf6ff07628bde33013657a31bd41ee60 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 10 Dec 2014 10:55:33 -0800 Subject: [PATCH 3/5] Fixes storagedriver Stat test Checks Stat on the directory before creating the file to make sure that it does not exist Properly cleans up after the test. --- storagedriver/testsuites/testsuites.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index bbdc6a1c4..d649a1732 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -598,10 +598,15 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { fileName := randomFilename(32) filePath := path.Join(dirPath, fileName) - defer suite.StorageDriver.Delete(dirPath) + defer suite.StorageDriver.Delete(firstPart(dirPath)) // Call on non-existent file/dir, check error. - fi, err := suite.StorageDriver.Stat(filePath) + fi, err := suite.StorageDriver.Stat(dirPath) + c.Assert(err, check.NotNil) + c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(fi, check.IsNil) + + fi, err = suite.StorageDriver.Stat(filePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) c.Assert(fi, check.IsNil) From 9297693675a1c0c93feb1c2a35f71fb4c57f71b4 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 10 Dec 2014 10:57:47 -0800 Subject: [PATCH 4/5] Improves storagedriver concurrency testing Creates trees instead of flat files for TestConcurrentFileStreams Adds TestConcurrentStreamReads, which writes a large file (smaller in Short mode), and then ensures that several concurrent readers properly read their portions of the file with random offsets --- storagedriver/testsuites/testsuites.go | 47 +++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index d649a1732..b568a9c8c 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -651,9 +651,46 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { } } +// TestConcurrentStreamReads checks that multiple clients can safely read from +// the same file simultaneously with various offsets. +func (suite *DriverSuite) TestConcurrentStreamReads(c *check.C) { + var filesize int64 = 128 * 1024 * 1024 + + if testing.Short() { + filesize = 10 * 1024 * 1024 + c.Log("Reducing file size to 10MB for short mode") + } + + filename := randomPath(32) + contents := randomContents(filesize) + + defer suite.StorageDriver.Delete(firstPart(filename)) + + err := suite.StorageDriver.PutContent(filename, contents) + c.Assert(err, check.IsNil) + + var wg sync.WaitGroup + + readContents := func() { + defer wg.Done() + offset := rand.Int63n(int64(len(contents))) + reader, err := suite.StorageDriver.ReadStream(filename, offset) + c.Assert(err, check.IsNil) + + readContents, err := ioutil.ReadAll(reader) + c.Assert(err, check.IsNil) + c.Assert(readContents, check.DeepEquals, contents[offset:]) + } + + wg.Add(10) + for i := 0; i < 10; i++ { + go readContents() + } + wg.Wait() +} + // TestConcurrentFileStreams checks that multiple *os.File objects can be passed // in to WriteStream concurrently without hanging. -// TODO(bbland): fix this test... func (suite *DriverSuite) TestConcurrentFileStreams(c *check.C) { // if _, isIPC := suite.StorageDriver.(*ipc.StorageDriverClient); isIPC { // c.Skip("Need to fix out-of-process concurrency") @@ -682,8 +719,8 @@ func (suite *DriverSuite) testFileStreams(c *check.C, size int64) { c.Assert(err, check.IsNil) defer os.Remove(tf.Name()) - tfName := path.Base(tf.Name()) - defer suite.StorageDriver.Delete(tfName) + filename := randomPath(32) + defer suite.StorageDriver.Delete(firstPart(filename)) contents := randomContents(size) @@ -693,11 +730,11 @@ func (suite *DriverSuite) testFileStreams(c *check.C, size int64) { tf.Sync() tf.Seek(0, os.SEEK_SET) - nn, err := suite.StorageDriver.WriteStream(tfName, 0, tf) + nn, err := suite.StorageDriver.WriteStream(filename, 0, tf) c.Assert(err, check.IsNil) c.Assert(nn, check.Equals, size) - reader, err := suite.StorageDriver.ReadStream(tfName, 0) + reader, err := suite.StorageDriver.ReadStream(filename, 0) c.Assert(err, check.IsNil) defer reader.Close() From 8a555bbb5fe171f28e1171712550ffc6f5755ce9 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Wed, 10 Dec 2014 11:09:14 -0800 Subject: [PATCH 5/5] Attempts to fix go vet for circle --- client/client_test.go | 60 +++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index d4a335ec7..5eaf6b97f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -90,18 +90,16 @@ func TestPush(t *testing.T) { } } - handler := testutil.NewHandler(append(blobRequestResponseMappings, testutil.RequestResponseMap{ - { - Request: testutil.Request{ - Method: "PUT", - Route: "/v2/" + name + "/manifest/" + tag, - Body: manifestBytes, - }, - Response: testutil.Response{ - StatusCode: http.StatusOK, - }, + handler := testutil.NewHandler(append(blobRequestResponseMappings, testutil.RequestResponseMapping{ + Request: testutil.Request{ + Method: "PUT", + Route: "/v2/" + name + "/manifest/" + tag, + Body: manifestBytes, }, - }...)) + Response: testutil.Response{ + StatusCode: http.StatusOK, + }, + })) server := httptest.NewServer(handler) client := New(server.URL) objectStore := &memoryObjectStore{ @@ -183,18 +181,16 @@ func TestPull(t *testing.T) { } } - handler := testutil.NewHandler(append(blobRequestResponseMappings, testutil.RequestResponseMap{ - { - Request: testutil.Request{ - Method: "GET", - Route: "/v2/" + name + "/manifest/" + tag, - }, - Response: testutil.Response{ - StatusCode: http.StatusOK, - Body: manifestBytes, - }, + handler := testutil.NewHandler(append(blobRequestResponseMappings, testutil.RequestResponseMapping{ + Request: testutil.Request{ + Method: "GET", + Route: "/v2/" + name + "/manifest/" + tag, }, - }...)) + Response: testutil.Response{ + StatusCode: http.StatusOK, + Body: manifestBytes, + }, + })) server := httptest.NewServer(handler) client := New(server.URL) objectStore := &memoryObjectStore{ @@ -306,18 +302,16 @@ func TestPullResume(t *testing.T) { } for i := 0; i < 3; i++ { - layerRequestResponseMappings = append(layerRequestResponseMappings, testutil.RequestResponseMap{ - { - Request: testutil.Request{ - Method: "GET", - Route: "/v2/" + name + "/manifest/" + tag, - }, - Response: testutil.Response{ - StatusCode: http.StatusOK, - Body: manifestBytes, - }, + layerRequestResponseMappings = append(layerRequestResponseMappings, testutil.RequestResponseMapping{ + Request: testutil.Request{ + Method: "GET", + Route: "/v2/" + name + "/manifest/" + tag, }, - }...) + Response: testutil.Response{ + StatusCode: http.StatusOK, + Body: manifestBytes, + }, + }) } handler := testutil.NewHandler(layerRequestResponseMappings)