From 66b4999765d22285bc80c597b0799952de2346fc Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 14 May 2017 12:56:10 +0200 Subject: [PATCH] tests: Remove code generation, use reflection This simplifies the code. --- src/restic/backend/test/benchmarks.go | 16 +- src/restic/backend/test/doc.go | 24 +-- src/restic/backend/test/funcs.go | 31 ---- src/restic/backend/test/generate_test_list.go | 152 ------------------ src/restic/backend/test/suite.go | 84 +++++++++- src/restic/backend/test/tests.go | 32 ++-- 6 files changed, 114 insertions(+), 225 deletions(-) delete mode 100644 src/restic/backend/test/funcs.go delete mode 100644 src/restic/backend/test/generate_test_list.go diff --git a/src/restic/backend/test/benchmarks.go b/src/restic/backend/test/benchmarks.go index c9b5691f6..2b2b0666d 100644 --- a/src/restic/backend/test/benchmarks.go +++ b/src/restic/backend/test/benchmarks.go @@ -24,9 +24,9 @@ func remove(t testing.TB, be restic.Backend, h restic.Handle) { } } -// BackendBenchmarkLoadFile benchmarks the Load() method of a backend by +// BenchmarkLoadFile benchmarks the Load() method of a backend by // loading a complete file. -func BackendBenchmarkLoadFile(t *testing.B, s *Suite) { +func (s *Suite) BenchmarkLoadFile(t *testing.B) { be := s.open(t) defer s.close(t, be) @@ -64,9 +64,9 @@ func BackendBenchmarkLoadFile(t *testing.B, s *Suite) { } } -// BackendBenchmarkLoadPartialFile benchmarks the Load() method of a backend by +// BenchmarkLoadPartialFile benchmarks the Load() method of a backend by // loading the remainder of a file starting at a given offset. -func BackendBenchmarkLoadPartialFile(t *testing.B, s *Suite) { +func (s *Suite) BenchmarkLoadPartialFile(t *testing.B) { be := s.open(t) defer s.close(t, be) @@ -107,9 +107,9 @@ func BackendBenchmarkLoadPartialFile(t *testing.B, s *Suite) { } } -// BackendBenchmarkLoadPartialFileOffset benchmarks the Load() method of a +// BenchmarkLoadPartialFileOffset benchmarks the Load() method of a // backend by loading a number of bytes of a file starting at a given offset. -func BackendBenchmarkLoadPartialFileOffset(t *testing.B, s *Suite) { +func (s *Suite) BenchmarkLoadPartialFileOffset(t *testing.B) { be := s.open(t) defer s.close(t, be) @@ -151,8 +151,8 @@ func BackendBenchmarkLoadPartialFileOffset(t *testing.B, s *Suite) { } } -// BackendBenchmarkSave benchmarks the Save() method of a backend. -func BackendBenchmarkSave(t *testing.B, s *Suite) { +// BenchmarkSave benchmarks the Save() method of a backend. +func (s *Suite) BenchmarkSave(t *testing.B) { be := s.open(t) defer s.close(t, be) diff --git a/src/restic/backend/test/doc.go b/src/restic/backend/test/doc.go index 61cb835a7..c1704d2c9 100644 --- a/src/restic/backend/test/doc.go +++ b/src/restic/backend/test/doc.go @@ -14,6 +14,14 @@ // // Assuming a *Suite is returned by newTestSuite(), the tests and benchmarks // can be run like this: +// func newTestSuite(t testing.TB) *test.Suite { +// return &test.Suite{ +// Create: func(cfg interface{}) (restic.Backend, error) { +// [...] +// }, +// [...] +// } +// } // // func TestSuiteBackendMem(t *testing.T) { // newTestSuite(t).RunTests(t) @@ -23,16 +31,12 @@ // newTestSuite(b).RunBenchmarks(b) // } // +// The functions are run in alphabetical order. +// // Add new tests // -// A new test or benchmark can be added by implementing a function with a name -// starting with BackendTest or BackendBenchmark. The first parameter is either -// a testing.TB or *testing.T (for tests), or a *testing.B (for benchmarks). -// The second parameter is a pointer to the test suite currently running -// (*Suite). -// -// The list of functions to run is contained in the file funcs.go, which is -// generated by generate_test_list.go. To regenerate that file, use go generate -// restic/backend/test. Test/benchmark functions are run in the order they are -// defined. +// A new test or benchmark can be added by implementing a method on *Suite +// with the name starting with "Test" and a single *testing.T parameter for +// test. For benchmarks, the name must start with "Benchmark" and the parameter +// is a *testing.B package test diff --git a/src/restic/backend/test/funcs.go b/src/restic/backend/test/funcs.go deleted file mode 100644 index 1f8a12a77..000000000 --- a/src/restic/backend/test/funcs.go +++ /dev/null @@ -1,31 +0,0 @@ -// DO NOT EDIT, AUTOMATICALLY GENERATED - -package test - -import ( - "testing" -) - -var testFunctions = []struct { - Name string - Fn func(testing.TB, *Suite) -}{ - {"CreateWithConfig", BackendTestCreateWithConfig}, - {"Location", BackendTestLocation}, - {"Config", BackendTestConfig}, - {"Load", BackendTestLoad}, - {"Save", BackendTestSave}, - {"SaveFilenames", BackendTestSaveFilenames}, - {"Backend", BackendTestBackend}, - {"Delete", BackendTestDelete}, -} - -var benchmarkFunctions = []struct { - Name string - Fn func(*testing.B, *Suite) -}{ - {"LoadFile", BackendBenchmarkLoadFile}, - {"LoadPartialFile", BackendBenchmarkLoadPartialFile}, - {"LoadPartialFileOffset", BackendBenchmarkLoadPartialFileOffset}, - {"Save", BackendBenchmarkSave}, -} diff --git a/src/restic/backend/test/generate_test_list.go b/src/restic/backend/test/generate_test_list.go deleted file mode 100644 index 98ae06fe4..000000000 --- a/src/restic/backend/test/generate_test_list.go +++ /dev/null @@ -1,152 +0,0 @@ -// +build ignore - -package main - -import ( - "bufio" - "bytes" - "flag" - "fmt" - "go/format" - "io" - "log" - "os" - "path/filepath" - "regexp" - "strings" - "text/template" - "unicode" - "unicode/utf8" -) - -var data struct { - Package string - TestFuncs []string - BenchmarkFuncs []string -} - -var testTemplate = ` -// DO NOT EDIT, AUTOMATICALLY GENERATED - -package {{ .Package }} - -import ( - "testing" -) - -var testFunctions = []struct { - Name string - Fn func(testing.TB, *Suite) -}{ -{{ range $f := .TestFuncs -}} - {"{{ $f }}", BackendTest{{ $f }},}, -{{ end }} -} - -var benchmarkFunctions = []struct { - Name string - Fn func(*testing.B, *Suite) -}{ -{{ range $f := .BenchmarkFuncs -}} - {"{{ $f }}", BackendBenchmark{{ $f }},}, -{{ end }} -} -` - -var testFiles = flag.String("testfiles", "tests.go,benchmarks.go", "files to search test functions in (comma separated)") -var outputFile = flag.String("output", "funcs.go", "output file to write generated code to") -var packageName = flag.String("package", "", "the package name to use") -var prefix = flag.String("prefix", "", "test function prefix") -var quiet = flag.Bool("quiet", false, "be quiet") - -func errx(err error) { - if err == nil { - return - } - - fmt.Fprintf(os.Stderr, "error: %v\n", err) - os.Exit(1) -} - -var testFuncRegex = regexp.MustCompile(`^func\s+BackendTest(.+)\s*\(`) -var benchmarkFuncRegex = regexp.MustCompile(`^func\s+BackendBenchmark(.+)\s*\(`) - -func findFunctions() (testFuncs, benchmarkFuncs []string) { - for _, filename := range strings.Split(*testFiles, ",") { - f, err := os.Open(filename) - errx(err) - - sc := bufio.NewScanner(f) - for sc.Scan() { - match := testFuncRegex.FindStringSubmatch(sc.Text()) - if len(match) > 0 { - testFuncs = append(testFuncs, match[1]) - } - - match = benchmarkFuncRegex.FindStringSubmatch(sc.Text()) - if len(match) > 0 { - benchmarkFuncs = append(benchmarkFuncs, match[1]) - } - } - - if err := sc.Err(); err != nil { - log.Fatalf("Error scanning file: %v", err) - } - - errx(f.Close()) - } - - return testFuncs, benchmarkFuncs -} - -func generateOutput(wr io.Writer, data interface{}) { - t := template.Must(template.New("backendtest").Parse(testTemplate)) - - buf := bytes.NewBuffer(nil) - errx(t.Execute(buf, data)) - - source, err := format.Source(buf.Bytes()) - errx(err) - - _, err = wr.Write(source) - errx(err) -} - -func packageTestFunctionPrefix(pkg string) string { - if pkg == "" { - return "" - } - - r, n := utf8.DecodeRuneInString(pkg) - return string(unicode.ToUpper(r)) + pkg[n:] -} - -func init() { - flag.Parse() -} - -func main() { - dir, err := os.Getwd() - if err != nil { - fmt.Fprintf(os.Stderr, "Getwd() %v\n", err) - os.Exit(1) - } - - pkg := *packageName - if pkg == "" { - pkg = filepath.Base(dir) - } - - f, err := os.Create(*outputFile) - errx(err) - - data.Package = pkg - data.TestFuncs, data.BenchmarkFuncs = findFunctions() - generateOutput(f, data) - - errx(f.Close()) - - if !*quiet { - fmt.Printf("wrote backend tests for package %v to %v\n", data.Package, *outputFile) - } -} diff --git a/src/restic/backend/test/suite.go b/src/restic/backend/test/suite.go index f7409aa4c..0b9b78a88 100644 --- a/src/restic/backend/test/suite.go +++ b/src/restic/backend/test/suite.go @@ -1,8 +1,10 @@ package test import ( + "reflect" "restic" "restic/test" + "strings" "testing" ) @@ -38,10 +40,8 @@ func (s *Suite) RunTests(t *testing.T) { be := s.create(t) s.close(t, be) - for _, test := range testFunctions { - t.Run(test.Name, func(t *testing.T) { - test.Fn(t, s) - }) + for _, test := range s.testFuncs(t) { + t.Run(test.Name, test.Fn) } if !test.TestCleanupTempDirs { @@ -54,6 +54,76 @@ func (s *Suite) RunTests(t *testing.T) { } } +type testFunction struct { + Name string + Fn func(*testing.T) +} + +func (s *Suite) testFuncs(t testing.TB) (funcs []testFunction) { + tpe := reflect.TypeOf(s) + v := reflect.ValueOf(s) + + for i := 0; i < tpe.NumMethod(); i++ { + methodType := tpe.Method(i) + name := methodType.Name + + // discard functions which do not have the right name + if !strings.HasPrefix(name, "Test") { + continue + } + + iface := v.Method(i).Interface() + f, ok := iface.(func(*testing.T)) + if !ok { + t.Logf("warning: function %v of *Suite has the wrong signature for a test function\nwant: func(*testing.T),\nhave: %T", + name, iface) + continue + } + + funcs = append(funcs, testFunction{ + Name: name, + Fn: f, + }) + } + + return funcs +} + +type benchmarkFunction struct { + Name string + Fn func(*testing.B) +} + +func (s *Suite) benchmarkFuncs(t testing.TB) (funcs []benchmarkFunction) { + tpe := reflect.TypeOf(s) + v := reflect.ValueOf(s) + + for i := 0; i < tpe.NumMethod(); i++ { + methodType := tpe.Method(i) + name := methodType.Name + + // discard functions which do not have the right name + if !strings.HasPrefix(name, "Benchmark") { + continue + } + + iface := v.Method(i).Interface() + f, ok := iface.(func(*testing.B)) + if !ok { + t.Logf("warning: function %v of *Suite has the wrong signature for a test function\nwant: func(*testing.T),\nhave: %T", + name, iface) + continue + } + + funcs = append(funcs, benchmarkFunction{ + Name: name, + Fn: f, + }) + } + + return funcs +} + // RunBenchmarks executes all defined benchmarks as subtests of b. func (s *Suite) RunBenchmarks(b *testing.B) { var err error @@ -66,10 +136,8 @@ func (s *Suite) RunBenchmarks(b *testing.B) { be := s.create(b) s.close(b, be) - for _, test := range benchmarkFunctions { - b.Run(test.Name, func(b *testing.B) { - test.Fn(b, s) - }) + for _, test := range s.benchmarkFuncs(b) { + b.Run(test.Name, test.Fn) } if !test.TestCleanupTempDirs { diff --git a/src/restic/backend/test/tests.go b/src/restic/backend/test/tests.go index 9110b0cf9..7c1031099 100644 --- a/src/restic/backend/test/tests.go +++ b/src/restic/backend/test/tests.go @@ -19,9 +19,9 @@ import ( "restic/backend" ) -// BackendTestCreateWithConfig tests that creating a backend in a location which already +// TestCreateWithConfig tests that creating a backend in a location which already // has a config file fails. -func BackendTestCreateWithConfig(t testing.TB, s *Suite) { +func (s *Suite) TestCreateWithConfig(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -52,8 +52,8 @@ func BackendTestCreateWithConfig(t testing.TB, s *Suite) { } } -// BackendTestLocation tests that a location string is returned. -func BackendTestLocation(t testing.TB, s *Suite) { +// TestLocation tests that a location string is returned. +func (s *Suite) TestLocation(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -63,8 +63,8 @@ func BackendTestLocation(t testing.TB, s *Suite) { } } -// BackendTestConfig saves and loads a config from the backend. -func BackendTestConfig(t testing.TB, s *Suite) { +// TestConfig saves and loads a config from the backend. +func (s *Suite) TestConfig(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -99,8 +99,8 @@ func BackendTestConfig(t testing.TB, s *Suite) { remove(t, b, restic.Handle{Type: restic.ConfigFile}) } -// BackendTestLoad tests the backend's Load function. -func BackendTestLoad(t testing.TB, s *Suite) { +// TestLoad tests the backend's Load function. +func (s *Suite) TestLoad(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -232,8 +232,8 @@ func (ec errorCloser) Size() int64 { return ec.size } -// BackendTestSave tests saving data in the backend. -func BackendTestSave(t testing.TB, s *Suite) { +// TestSave tests saving data in the backend. +func (s *Suite) TestSave(t *testing.T) { b := s.open(t) defer s.close(t, b) var id restic.ID @@ -332,8 +332,8 @@ var filenameTests = []struct { }, } -// BackendTestSaveFilenames tests saving data with various file names in the backend. -func BackendTestSaveFilenames(t testing.TB, s *Suite) { +// TestSaveFilenames tests saving data with various file names in the backend. +func (s *Suite) TestSaveFilenames(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -381,8 +381,8 @@ func store(t testing.TB, b restic.Backend, tpe restic.FileType, data []byte) res return h } -// BackendTestBackend tests all functions of the backend. -func BackendTestBackend(t testing.TB, s *Suite) { +// TestBackend tests all functions of the backend. +func (s *Suite) TestBackend(t *testing.T) { b := s.open(t) defer s.close(t, b) @@ -515,8 +515,8 @@ func BackendTestBackend(t testing.TB, s *Suite) { } } -// BackendTestDelete tests the Delete function. -func BackendTestDelete(t testing.TB, s *Suite) { +// TestDelete tests the Delete function. +func (s *Suite) TestDelete(t *testing.T) { if !test.TestCleanupTempDirs { t.Skipf("not removing backend, TestCleanupTempDirs is false") }