3448: update tests with "untouchable" volumes

Added a "Touchable" flag to MockVolume, so that we can test that
PutBlock does the right thing when Touch fails.  Previous code was using
volume.Bad as an attempt to test this, but that short-circuited the Put
request before it ever attempted to call Touch.

Added TODOs with more testing improvements we can make as time permits.

diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 2b7e411aaf1f8c560da93af7fb4c770b60363fde..686f502500cec1e6e7ad3508f2978ef55522a6dc 100644 (file)
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -266,12 +266,13 @@ func TestPutBlockTouchFails(t *testing.T) {
                t.Fatalf("vols[0].Mtime(%s): %s\n", TEST_HASH, err)
        }
 
-       // Mark the volume bad and call PutBlock.
-       vols[0].(*MockVolume).Bad = true
+       // vols[0].Touch will fail on the next call, so the volume
+       // manager will store a copy on vols[1] instead.
+       vols[0].(*MockVolume).Touchable = false
        if err := PutBlock(TEST_BLOCK, TEST_HASH); err != nil {
                t.Fatalf("PutBlock: %v", err)
        }
-       vols[0].(*MockVolume).Bad = false
+       vols[0].(*MockVolume).Touchable = true
 
        // Now the mtime on the block on vols[0] should be unchanged, and
        // there should be a copy of the block on vols[1].
@@ -280,7 +281,7 @@ func TestPutBlockTouchFails(t *testing.T) {
                t.Fatalf("vols[0].Mtime(%s): %s\n", TEST_HASH, err)
        }
        if !new_mtime.Equal(old_mtime) {
-               t.Errorf("bad block mtimes do not match:\nold_mtime = %v\nnew_mtime = %v\n",
+               t.Errorf("mtime was changed on vols[0]:\nold_mtime = %v\nnew_mtime = %v\n",
                        old_mtime, new_mtime)
        }
        result, err := vols[1].Get(TEST_HASH)

diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index 6fb1a1e0876324aac78046932dbfe511f3384cf4..e7683ee991a41ac8a8f45cfa9f5f263e2a7a42a6 100644 (file)
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -28,17 +28,29 @@ type Volume interface {
 // If the Bad field is true, this volume should return an error
 // on all writes and puts.
 //
+// The Touchable field signifies whether the Touch method will
+// succeed.  Defaults to true.  Note that Bad and Touchable are
+// independent: a MockVolume may be set up so that Put fails but Touch
+// works or vice versa.
+//
+// TODO(twp): rename Bad to something more descriptive, e.g. Writable,
+// and make sure that the tests that rely on it are testing the right
+// thing.  We may need to simulate Writable, Touchable and Corrupt
+// volumes in different ways.
+//
 type MockVolume struct {
        Store      map[string][]byte
        Timestamps map[string]time.Time
        Bad        bool
+       Touchable  bool
 }
 
 func CreateMockVolume() *MockVolume {
        return &MockVolume{
-               make(map[string][]byte),
-               make(map[string]time.Time),
-               false,
+               Store:      make(map[string][]byte),
+               Timestamps: make(map[string]time.Time),
+               Bad:        false,
+               Touchable:  true,
        }
 }
 
@@ -60,11 +72,11 @@ func (v *MockVolume) Put(loc string, block []byte) error {
 }
 
 func (v *MockVolume) Touch(loc string) error {
-       if v.Bad {
-               return errors.New("Bad volume")
+       if v.Touchable {
+               v.Timestamps[loc] = time.Now()
+               return nil
        }
-       v.Timestamps[loc] = time.Now()
-       return nil
+       return errors.New("Touch failed")
 }
 
 func (v *MockVolume) Mtime(loc string) (time.Time, error) {

diff --git a/services/keepstore/volume_unix_test.go b/services/keepstore/volume_unix_test.go
index 7a1c06c8161e0c4c8d9e8a8681aa7a510371643b..d6aeac618582a3c555d92b1515654fc8386a6795 100644 (file)
--- a/services/keepstore/volume_unix_test.go
+++ b/services/keepstore/volume_unix_test.go
@@ -119,7 +119,11 @@ func TestPutTouch(t *testing.T) {
        }
        // Sleep for 1s, then put the block again.  The volume
        // should report a more recent mtime.
+       //
        // TODO(twp): this would be better handled with a mock Time object.
+       // Alternatively, set the mtime manually to some moment in the past
+       // (maybe a v.SetMtime method?)
+       //
        time.Sleep(time.Second)
        if err := v.Put(TEST_HASH, TEST_BLOCK); err != nil {
                t.Error(err)