From 7ec0543af31f279e00f23aa862b7b822714c8fb2 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 09:43:28 +0200 Subject: [PATCH 1/8] testing: Add id to error message in panic --- src/restic/testing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restic/testing.go b/src/restic/testing.go index 9df54e7b6..144f53bd1 100644 --- a/src/restic/testing.go +++ b/src/restic/testing.go @@ -205,7 +205,7 @@ func TestCreateSnapshot(t testing.TB, repo Repository, at time.Time, depth int, func TestParseID(s string) ID { id, err := ParseID(s) if err != nil { - panic(err) + panic(fmt.Sprintf("unable to parse string %q as ID: %v", s, err)) } return id From 79477fdfe418d5ca8c2dbb78f0502cbbb76673f2 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 10:16:29 +0200 Subject: [PATCH 2/8] backend/test: Randomize test suite --- src/restic/backend/test/tests.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index 9919220b9..1e6213ee2 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -13,12 +13,19 @@ import ( "sort" "strings" "testing" + "time" "restic/test" "restic/backend" ) +func seedRand(t testing.TB) { + seed := time.Now().UnixNano() + rand.Seed(seed) + t.Logf("rand initialized with seed %d", seed) +} + // TestCreateWithConfig tests that creating a backend in a location which already // has a config file fails. func (s *Suite) TestCreateWithConfig(t *testing.T) { @@ -101,6 +108,8 @@ func (s *Suite) TestConfig(t *testing.T) { // TestLoad tests the backend's Load function. func (s *Suite) TestLoad(t *testing.T) { + seedRand(t) + b := s.open(t) defer s.close(t, b) @@ -160,6 +169,7 @@ func (s *Suite) TestLoad(t *testing.T) { d = d[:l] } + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) rd, err := b.Load(handle, getlen, int64(o)) if err != nil { t.Errorf("Load(%d, %d) returned unexpected error: %+v", l, o, err) @@ -234,6 +244,8 @@ func (ec errorCloser) Size() int64 { // TestSave tests saving data in the backend. func (s *Suite) TestSave(t *testing.T) { + seedRand(t) + b := s.open(t) defer s.close(t, b) var id restic.ID From 24ec14738dcaea5f497cd4e54795a1342644bb46 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 12:32:42 +0200 Subject: [PATCH 3/8] backend/test: Skip offset == length test --- src/restic/backend/test/tests.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index 1e6213ee2..f63d9c336 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -156,8 +156,8 @@ func (s *Suite) TestLoad(t *testing.T) { if o < len(d) { d = d[o:] } else { - o = len(d) - d = d[:0] + t.Logf("offset == length, skipping test") + continue } getlen := l From 8395b534004a600ebd2f97ef54a417ee8b89e4c4 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 12:32:53 +0200 Subject: [PATCH 4/8] backend/test: Reduce verbosity in logs --- src/restic/backend/test/tests.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index f63d9c336..59d6d9dfd 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -134,6 +134,8 @@ func (s *Suite) TestLoad(t *testing.T) { t.Fatalf("Save() error: %+v", err) } + t.Logf("saved %d bytes as %v", length, handle) + rd, err := b.Load(handle, 100, -1) if err == nil { t.Fatalf("Load() returned no error for negative offset!") @@ -169,15 +171,16 @@ func (s *Suite) TestLoad(t *testing.T) { d = d[:l] } - t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) rd, err := b.Load(handle, getlen, int64(o)) if err != nil { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) returned unexpected error: %+v", l, o, err) continue } buf, err := ioutil.ReadAll(rd) if err != nil { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) ReadAll() returned unexpected error: %+v", l, o, err) if err = rd.Close(); err != nil { t.Errorf("Load(%d, %d) rd.Close() returned error: %+v", l, o, err) @@ -186,6 +189,7 @@ func (s *Suite) TestLoad(t *testing.T) { } if l == 0 && len(buf) != len(d) { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) wrong number of bytes read: want %d, got %d", l, o, len(d), len(buf)) if err = rd.Close(); err != nil { t.Errorf("Load(%d, %d) rd.Close() returned error: %+v", l, o, err) @@ -194,6 +198,7 @@ func (s *Suite) TestLoad(t *testing.T) { } if l > 0 && l <= len(d) && len(buf) != l { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) wrong number of bytes read: want %d, got %d", l, o, l, len(buf)) if err = rd.Close(); err != nil { t.Errorf("Load(%d, %d) rd.Close() returned error: %+v", l, o, err) @@ -202,6 +207,7 @@ func (s *Suite) TestLoad(t *testing.T) { } if l > len(d) && len(buf) != len(d) { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) wrong number of bytes read for overlong read: want %d, got %d", l, o, l, len(buf)) if err = rd.Close(); err != nil { t.Errorf("Load(%d, %d) rd.Close() returned error: %+v", l, o, err) @@ -210,6 +216,7 @@ func (s *Suite) TestLoad(t *testing.T) { } if !bytes.Equal(buf, d) { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) returned wrong bytes", l, o) if err = rd.Close(); err != nil { t.Errorf("Load(%d, %d) rd.Close() returned error: %+v", l, o, err) @@ -219,6 +226,7 @@ func (s *Suite) TestLoad(t *testing.T) { err = rd.Close() if err != nil { + t.Logf("Load, l %v, o %v, len(d) %v, getlen %v", l, o, len(d), getlen) t.Errorf("Load(%d, %d) rd.Close() returned unexpected error: %+v", l, o, err) continue } From e046a2a6da8277f3a71afdd30986159175ec61f8 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 10:41:57 +0200 Subject: [PATCH 5/8] sftp: Use path instead of filepath --- src/restic/backend/sftp/sftp.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/restic/backend/sftp/sftp.go b/src/restic/backend/sftp/sftp.go index 3eedfa76d..8070d01fe 100644 --- a/src/restic/backend/sftp/sftp.go +++ b/src/restic/backend/sftp/sftp.go @@ -7,7 +7,6 @@ import ( "os" "os/exec" "path" - "path/filepath" "restic" "strings" "time" @@ -416,7 +415,7 @@ func (r *SFTP) List(t restic.FileType, done <-chan struct{}) <-chan string { } select { - case ch <- filepath.Base(walker.Path()): + case ch <- path.Base(walker.Path()): case <-done: return } From 1f5954e2c10318518d9f2e2aeb723cdbc8f6e86e Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 11:06:48 +0200 Subject: [PATCH 6/8] layout: Test DefaultLayout for empty path prefix --- src/restic/backend/layout_default.go | 2 +- src/restic/backend/layout_test.go | 89 ++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/restic/backend/layout_default.go b/src/restic/backend/layout_default.go index 836700b33..665b85531 100644 --- a/src/restic/backend/layout_default.go +++ b/src/restic/backend/layout_default.go @@ -34,7 +34,7 @@ func (l *DefaultLayout) Dirname(h restic.Handle) string { func (l *DefaultLayout) Filename(h restic.Handle) string { name := h.Name if h.Type == restic.ConfigFile { - name = "config" + return l.Join(l.Path, "config") } return l.Join(l.Dirname(h), name) diff --git a/src/restic/backend/layout_test.go b/src/restic/backend/layout_test.go index cd06d4e63..48bc2a3db 100644 --- a/src/restic/backend/layout_test.go +++ b/src/restic/backend/layout_test.go @@ -12,53 +12,103 @@ import ( ) func TestDefaultLayout(t *testing.T) { - path, cleanup := TempDir(t) + tempdir, cleanup := TempDir(t) defer cleanup() var tests = []struct { + path string + join func(...string) string restic.Handle filename string }{ { + tempdir, + filepath.Join, restic.Handle{Type: restic.DataFile, Name: "0123456"}, - filepath.Join(path, "data", "01", "0123456"), + filepath.Join(tempdir, "data", "01", "0123456"), }, { + tempdir, + filepath.Join, restic.Handle{Type: restic.ConfigFile, Name: "CFG"}, - filepath.Join(path, "config"), + filepath.Join(tempdir, "config"), }, { + tempdir, + filepath.Join, restic.Handle{Type: restic.SnapshotFile, Name: "123456"}, - filepath.Join(path, "snapshots", "123456"), + filepath.Join(tempdir, "snapshots", "123456"), }, { + tempdir, + filepath.Join, restic.Handle{Type: restic.IndexFile, Name: "123456"}, - filepath.Join(path, "index", "123456"), + filepath.Join(tempdir, "index", "123456"), }, { + tempdir, + filepath.Join, restic.Handle{Type: restic.LockFile, Name: "123456"}, - filepath.Join(path, "locks", "123456"), + filepath.Join(tempdir, "locks", "123456"), }, { + tempdir, + filepath.Join, restic.Handle{Type: restic.KeyFile, Name: "123456"}, - filepath.Join(path, "keys", "123456"), + filepath.Join(tempdir, "keys", "123456"), + }, + { + "", + path.Join, + restic.Handle{Type: restic.DataFile, Name: "0123456"}, + "data/01/0123456", + }, + { + "", + path.Join, + restic.Handle{Type: restic.ConfigFile, Name: "CFG"}, + "config", + }, + { + "", + path.Join, + restic.Handle{Type: restic.SnapshotFile, Name: "123456"}, + "snapshots/123456", + }, + { + "", + path.Join, + restic.Handle{Type: restic.IndexFile, Name: "123456"}, + "index/123456", + }, + { + "", + path.Join, + restic.Handle{Type: restic.LockFile, Name: "123456"}, + "locks/123456", + }, + { + "", + path.Join, + restic.Handle{Type: restic.KeyFile, Name: "123456"}, + "keys/123456", }, - } - - l := &DefaultLayout{ - Path: path, - Join: filepath.Join, } t.Run("Paths", func(t *testing.T) { + l := &DefaultLayout{ + Path: tempdir, + Join: filepath.Join, + } + dirs := l.Paths() want := []string{ - filepath.Join(path, "data"), - filepath.Join(path, "snapshots"), - filepath.Join(path, "index"), - filepath.Join(path, "locks"), - filepath.Join(path, "keys"), + filepath.Join(tempdir, "data"), + filepath.Join(tempdir, "snapshots"), + filepath.Join(tempdir, "index"), + filepath.Join(tempdir, "locks"), + filepath.Join(tempdir, "keys"), } sort.Sort(sort.StringSlice(want)) @@ -71,6 +121,11 @@ func TestDefaultLayout(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("%v/%v", test.Type, test.Handle.Name), func(t *testing.T) { + l := &DefaultLayout{ + Path: test.path, + Join: test.join, + } + filename := l.Filename(test.Handle) if filename != test.filename { t.Fatalf("wrong filename, want %v, got %v", test.filename, filename) From c5244abad9f9b478d1a9d16c4425a994a8846b4b Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 12:31:19 +0200 Subject: [PATCH 7/8] rest: Improve error messages --- src/restic/backend/rest/rest.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/restic/backend/rest/rest.go b/src/restic/backend/rest/rest.go index db1118077..cb3fdadc8 100644 --- a/src/restic/backend/rest/rest.go +++ b/src/restic/backend/rest/rest.go @@ -127,9 +127,8 @@ func (b *restBackend) Save(h restic.Handle, rd io.Reader) (err error) { return errors.Wrap(err, "client.Post") } - // fmt.Printf("status is %v (%v)\n", resp.Status, resp.StatusCode) if resp.StatusCode != 200 { - return errors.Errorf("unexpected HTTP response code %v", resp.StatusCode) + return errors.Errorf("server response unexpected: %v (%v)", resp.Status, resp.StatusCode) } return nil @@ -179,7 +178,7 @@ func (b *restBackend) Load(h restic.Handle, length int, offset int64) (io.ReadCl if resp.StatusCode != 200 && resp.StatusCode != 206 { io.Copy(ioutil.Discard, resp.Body) resp.Body.Close() - return nil, errors.Errorf("unexpected HTTP response code %v", resp.StatusCode) + return nil, errors.Errorf("unexpected HTTP response (%v): %v", resp.StatusCode, resp.Status) } return resp.Body, nil @@ -204,7 +203,7 @@ func (b *restBackend) Stat(h restic.Handle) (restic.FileInfo, error) { } if resp.StatusCode != 200 { - return restic.FileInfo{}, errors.Errorf("unexpected HTTP response code %v", resp.StatusCode) + return restic.FileInfo{}, errors.Errorf("unexpected HTTP response (%v): %v", resp.StatusCode, resp.Status) } if resp.ContentLength < 0 { From f3f6924b61eb96113dde1e13b3f53631c3b29f50 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 28 May 2017 13:05:35 +0200 Subject: [PATCH 8/8] backend/test: Loose requirement about early error --- src/restic/backend/test/tests.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index 59d6d9dfd..a6f47452b 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -113,12 +113,15 @@ func (s *Suite) TestLoad(t *testing.T) { b := s.open(t) defer s.close(t, b) - _, err := b.Load(restic.Handle{}, 0, 0) + rd, err := b.Load(restic.Handle{}, 0, 0) if err == nil { t.Fatalf("Load() did not return an error for invalid handle") } + if rd != nil { + rd.Close() + } - _, err = b.Load(restic.Handle{Type: restic.DataFile, Name: "foobar"}, 0, 0) + err = testLoad(b, restic.Handle{Type: restic.DataFile, Name: "foobar"}, 0, 0) if err == nil { t.Fatalf("Load() did not return an error for non-existing blob") } @@ -136,7 +139,7 @@ func (s *Suite) TestLoad(t *testing.T) { t.Logf("saved %d bytes as %v", length, handle) - rd, err := b.Load(handle, 100, -1) + rd, err = b.Load(handle, 100, -1) if err == nil { t.Fatalf("Load() returned no error for negative offset!") } @@ -416,6 +419,21 @@ func store(t testing.TB, b restic.Backend, tpe restic.FileType, data []byte) res return h } +// testLoad loads a blob (but discards its contents). +func testLoad(b restic.Backend, h restic.Handle, length int, offset int64) error { + rd, err := b.Load(h, 0, 0) + if err != nil { + return err + } + + _, err = io.Copy(ioutil.Discard, rd) + cerr := rd.Close() + if err == nil { + err = cerr + } + return err +} + // TestBackend tests all functions of the backend. func (s *Suite) TestBackend(t *testing.T) { b := s.open(t) @@ -441,8 +459,8 @@ func (s *Suite) TestBackend(t *testing.T) { test.Assert(t, err != nil, "blob data could be extracted before creation") // try to read not existing blob - _, err = b.Load(h, 0, 0) - test.Assert(t, err != nil, "blob reader could be obtained before creation") + err = testLoad(b, h, 0, 0) + test.Assert(t, err != nil, "blob could be read before creation") // try to get string out, should fail ret, err = b.Test(h)