From 7f5523b8da9b1f8575fdcb42a173e000393d332b Mon Sep 17 00:00:00 2001 From: Oleg Butuzov Date: Tue, 18 May 2021 13:21:30 +0300 Subject: [PATCH 1/3] refactoring: code refactoring This code is being refactored, in order to fix race conditions in tests and make code base more tests-friendly. --- gotests.go | 6 +- internal/output/helpers.go | 8 + internal/output/imports.go | 6 + internal/output/options.go | 99 ++++++++++++ .../{output_test.go => options_test.go} | 0 internal/output/output.go | 94 ------------ internal/render/README.md | 0 internal/render/helpers.go | 106 +++++++++++++ internal/render/render.go | 145 ++++++------------ 9 files changed, 267 insertions(+), 197 deletions(-) create mode 100644 internal/output/helpers.go create mode 100644 internal/output/imports.go create mode 100755 internal/output/options.go rename internal/output/{output_test.go => options_test.go} (100%) mode change 100644 => 100755 delete mode 100644 internal/output/output.go mode change 100644 => 100755 internal/render/README.md create mode 100755 internal/render/helpers.go mode change 100644 => 100755 internal/render/render.go diff --git a/gotests.go b/gotests.go index fc4acfc..7cff8a0 100644 --- a/gotests.go +++ b/gotests.go @@ -120,7 +120,7 @@ func generateTest(src models.Path, files []models.Path, opt *Options) (*Generate return nil, nil } - b, err := output.Process(h, funcs, &output.Options{ + options := output.Options{ PrintInputs: opt.PrintInputs, Subtests: opt.Subtests, Parallel: opt.Parallel, @@ -129,7 +129,9 @@ func generateTest(src models.Path, files []models.Path, opt *Options) (*Generate TemplateDir: opt.TemplateDir, TemplateParams: opt.TemplateParams, TemplateData: opt.TemplateData, - }) + } + + b, err := options.Process(h, funcs) if err != nil { return nil, fmt.Errorf("output.Process: %v", err) } diff --git a/internal/output/helpers.go b/internal/output/helpers.go new file mode 100644 index 0000000..2ea3d0f --- /dev/null +++ b/internal/output/helpers.go @@ -0,0 +1,8 @@ +package output + +import "os" + +func IsFileExist(path string) bool { + _, err := os.Stat(path) + return !os.IsNotExist(err) +} diff --git a/internal/output/imports.go b/internal/output/imports.go new file mode 100644 index 0000000..5de0529 --- /dev/null +++ b/internal/output/imports.go @@ -0,0 +1,6 @@ +package output + +// we do not need support for aliases in import for now. +var importsMap = map[string]string{ + "testify": "github.com/stretchr/testify/assert", +} diff --git a/internal/output/options.go b/internal/output/options.go new file mode 100755 index 0000000..2572f3a --- /dev/null +++ b/internal/output/options.go @@ -0,0 +1,99 @@ +package output + +import ( + "bufio" + "bytes" + "fmt" + "io" + "io/ioutil" + "os" + + "github.com/cweill/gotests/internal/models" + "github.com/cweill/gotests/internal/render" + "golang.org/x/tools/imports" +) + +type Options struct { + PrintInputs bool + Subtests bool + Parallel bool + Named bool + Template string + TemplateDir string + TemplateParams map[string]interface{} + TemplateData [][]byte + + render *render.Render +} + +func (o *Options) Process(head *models.Header, funcs []*models.Function) ([]byte, error) { + o.render = render.New() + + switch { + case o.providesTemplateDir(): + if err := o.render.LoadCustomTemplates(o.TemplateDir); err != nil { + return nil, fmt.Errorf("loading custom templates: %v", err) + } + case o.providesTemplate(): + if err := o.render.LoadCustomTemplatesName(o.Template); err != nil { + return nil, fmt.Errorf("loading custom templates of name: %v", err) + } + case o.providesTemplateData(): + o.render.LoadFromData(o.TemplateData) + } + + // + tf, err := ioutil.TempFile("", "gotests_") + if err != nil { + return nil, fmt.Errorf("ioutil.TempFile: %v", err) + } + defer tf.Close() + defer os.Remove(tf.Name()) + + // create physical copy of test + b := &bytes.Buffer{} + if err := o.writeTests(b, head, funcs); err != nil { + return nil, err + } + + // format file + out, err := imports.Process(tf.Name(), b.Bytes(), nil) + if err != nil { + return nil, fmt.Errorf("imports.Process: %v", err) + } + return out, nil +} + +func (o *Options) providesTemplateData() bool { + return o != nil && len(o.TemplateData) > 0 +} + +func (o *Options) providesTemplateDir() bool { + return o != nil && o.TemplateDir != "" +} + +func (o *Options) providesTemplate() bool { + return o != nil && o.Template != "" +} + +func (o *Options) writeTests(w io.Writer, head *models.Header, funcs []*models.Function) error { + if path, ok := importsMap[o.Template]; ok { + head.Imports = append(head.Imports, &models.Import{ + Path: fmt.Sprintf(`"%s"`, path), + }) + } + + b := bufio.NewWriter(w) + if err := o.render.Header(b, head); err != nil { + return fmt.Errorf("render.Header: %v", err) + } + + for _, fun := range funcs { + err := o.render.TestFunction(b, fun, o.PrintInputs, o.Subtests, o.Named, o.Parallel, o.TemplateParams) + if err != nil { + return fmt.Errorf("render.TestFunction: %v", err) + } + } + + return b.Flush() +} diff --git a/internal/output/output_test.go b/internal/output/options_test.go old mode 100644 new mode 100755 similarity index 100% rename from internal/output/output_test.go rename to internal/output/options_test.go diff --git a/internal/output/output.go b/internal/output/output.go deleted file mode 100644 index f3fe351..0000000 --- a/internal/output/output.go +++ /dev/null @@ -1,94 +0,0 @@ -package output - -import ( - "bufio" - "bytes" - "fmt" - "io" - "io/ioutil" - "os" - - "golang.org/x/tools/imports" - - "github.com/cweill/gotests/internal/models" - "github.com/cweill/gotests/internal/render" -) - -// we do not need support for aliases in import for now. -var importsMap = map[string]string{ - "testify": "github.com/stretchr/testify/assert", -} - -type Options struct { - PrintInputs bool - Subtests bool - Parallel bool - Named bool - Template string - TemplateDir string - TemplateParams map[string]interface{} - TemplateData [][]byte -} - -func (o *Options) providesTemplateData() bool { return o != nil && len(o.TemplateData) > 0 } -func (o *Options) providesTemplateDir() bool { return o != nil && o.TemplateDir != "" } -func (o *Options) providesTemplate() bool { return o != nil && o.Template != "" } - -func Process(head *models.Header, funcs []*models.Function, opt *Options) ([]byte, error) { - switch { - case opt.providesTemplateDir(): - if err := render.LoadCustomTemplates(opt.TemplateDir); err != nil { - return nil, fmt.Errorf("loading custom templates: %v", err) - } - case opt.providesTemplate(): - if err := render.LoadCustomTemplatesName(opt.Template); err != nil { - return nil, fmt.Errorf("loading custom templates of name: %v", err) - } - case opt.providesTemplateData(): - render.LoadFromData(opt.TemplateData) - default: - render.Reset() - } - - tf, err := ioutil.TempFile("", "gotests_") - if err != nil { - return nil, fmt.Errorf("ioutil.TempFile: %v", err) - } - defer tf.Close() - defer os.Remove(tf.Name()) - b := &bytes.Buffer{} - if err := writeTests(b, head, funcs, opt); err != nil { - return nil, err - } - - out, err := imports.Process(tf.Name(), b.Bytes(), nil) - if err != nil { - return nil, fmt.Errorf("imports.Process: %v", err) - } - return out, nil -} - -func IsFileExist(path string) bool { - _, err := os.Stat(path) - return !os.IsNotExist(err) -} - -func writeTests(w io.Writer, head *models.Header, funcs []*models.Function, opt *Options) error { - if path, ok := importsMap[opt.Template]; ok { - head.Imports = append(head.Imports, &models.Import{ - Path: fmt.Sprintf(`"%s"`, path), - }) - } - - b := bufio.NewWriter(w) - if err := render.Header(b, head); err != nil { - return fmt.Errorf("render.Header: %v", err) - } - - for _, fun := range funcs { - if err := render.TestFunction(b, fun, opt.PrintInputs, opt.Subtests, opt.Named, opt.Parallel, opt.TemplateParams); err != nil { - return fmt.Errorf("render.TestFunction: %v", err) - } - } - return b.Flush() -} diff --git a/internal/render/README.md b/internal/render/README.md old mode 100644 new mode 100755 diff --git a/internal/render/helpers.go b/internal/render/helpers.go new file mode 100755 index 0000000..02e00c7 --- /dev/null +++ b/internal/render/helpers.go @@ -0,0 +1,106 @@ +package render + +//go:generate esc -o bindata/esc.go -pkg=bindata templates +import ( + "fmt" + "io" + "strings" + "text/template" + + "github.com/cweill/gotests/internal/models" +) + +const ( + name = "name" + nFile = 7 +) + +var tmpls *template.Template + +func fieldName(f *models.Field) string { + var n string + if f.IsNamed() { + n = f.Name + } else { + n = f.Type.String() + } + return n +} + +func receiverName(f *models.Receiver) string { + var n string + if f.IsNamed() { + n = f.Name + } else { + n = f.ShortName() + } + if n == "name" { + // Avoid conflict with test struct's "name" field. + n = "n" + } else if n == "t" { + // Avoid conflict with test argument. + // "tr" is short for t receiver. + n = "tr" + } + return n +} + +func parameterName(f *models.Field) string { + var n string + if f.IsNamed() { + n = f.Name + } else { + n = fmt.Sprintf("in%v", f.Index) + } + return n +} + +func wantName(f *models.Field) string { + var n string + if f.IsNamed() { + n = "want" + strings.Title(f.Name) + } else if f.Index == 0 { + n = "want" + } else { + n = fmt.Sprintf("want%v", f.Index) + } + return n +} + +func gotName(f *models.Field) string { + var n string + if f.IsNamed() { + n = "got" + strings.Title(f.Name) + } else if f.Index == 0 { + n = "got" + } else { + n = fmt.Sprintf("got%v", f.Index) + } + return n +} + +func Header(w io.Writer, h *models.Header) error { + if err := tmpls.ExecuteTemplate(w, "header", h); err != nil { + return err + } + _, err := w.Write(h.Code) + return err +} + +func TestFunction(w io.Writer, f *models.Function, printInputs, subtests, named, parallel bool, templateParams map[string]interface{}) error { + return tmpls.ExecuteTemplate(w, "function", struct { + *models.Function + PrintInputs bool + Subtests bool + Parallel bool + Named bool + TemplateParams map[string]interface{} + }{ + Function: f, + PrintInputs: printInputs, + Subtests: subtests, + Parallel: parallel, + Named: named, + TemplateParams: templateParams, + }) +} diff --git a/internal/render/render.go b/internal/render/render.go old mode 100644 new mode 100755 index ea58885..cedcd9b --- a/internal/render/render.go +++ b/internal/render/render.go @@ -1,12 +1,10 @@ package render -//go:generate esc -o bindata/esc.go -pkg=bindata templates import ( "fmt" "io" "io/ioutil" "path" - "strings" "text/template" "github.com/cweill/gotests/internal/models" @@ -14,36 +12,35 @@ import ( "github.com/cweill/gotests/templates" ) -const ( - name = "name" - nFile = 7 -) - -var tmpls *template.Template - -func init() { - Reset() +type Render struct { + tmpls *template.Template } -func Reset() { - initEmptyTmpls() - for _, name := range bindata.AssetNames() { - tmpls = template.Must(tmpls.Parse(bindata.FSMustString(false, name))) +func New() *Render { + r := Render{ + tmpls: template.New("render").Funcs(map[string]interface{}{ + "Field": fieldName, + "Receiver": receiverName, + "Param": parameterName, + "Want": wantName, + "Got": gotName, + }), } -} -// LoadFromData allows to load from a data slice -func LoadFromData(templateData [][]byte) { - initEmptyTmpls() - for _, d := range templateData { - tmpls = template.Must(tmpls.Parse(string(d))) + // default templates first + for _, name := range bindata.AssetNames() { + // if strings.Contains(bindata.FSMustString(false, name), "function") { + // fmt.Printf("render.New - bindata.[%s]\n", name) + // // fmt.Printf("> %s\n", bindata.FSMustString(false, name)) + // } + r.tmpls = template.Must(r.tmpls.Parse(bindata.FSMustString(false, name))) } + + return &r } // LoadCustomTemplates allows to load in custom templates from a specified path. -func LoadCustomTemplates(dir string) error { - initEmptyTmpls() - +func (r *Render) LoadCustomTemplates(dir string) error { files, err := ioutil.ReadDir(dir) if err != nil { return fmt.Errorf("ioutil.ReadDir: %v", err) @@ -53,15 +50,19 @@ func LoadCustomTemplates(dir string) error { for _, f := range files { templateFiles = append(templateFiles, path.Join(dir, f.Name())) } - tmpls, err = tmpls.ParseFiles(templateFiles...) + + // fmt.Printf("template(s)- %#v\n", templateFiles) + + r.tmpls, err = r.tmpls.ParseFiles(templateFiles...) if err != nil { return fmt.Errorf("tmpls.ParseFiles: %v", err) } + return nil } // LoadCustomTemplatesName allows to load in custom templates of a specified name from the templates directory. -func LoadCustomTemplatesName(name string) error { +func (r *Render) LoadCustomTemplatesName(name string) error { f, err := templates.Dir(false, "/").Open(name) if err != nil { return fmt.Errorf("templates.Open: %v", err) @@ -78,97 +79,39 @@ func LoadCustomTemplatesName(name string) error { return fmt.Errorf("templates.FSString: %v", err) } - tmpls, err = tmpls.Parse(text) + tmpls, err = r.tmpls.Parse(text) if err != nil { return fmt.Errorf("tmpls.Parse: %v", err) } + r.tmpls = tmpls } return nil } -func initEmptyTmpls() { - tmpls = template.New("render").Funcs(map[string]interface{}{ - "Field": fieldName, - "Receiver": receiverName, - "Param": parameterName, - "Want": wantName, - "Got": gotName, - }) -} - -func fieldName(f *models.Field) string { - var n string - if f.IsNamed() { - n = f.Name - } else { - n = f.Type.String() - } - return n -} - -func receiverName(f *models.Receiver) string { - var n string - if f.IsNamed() { - n = f.Name - } else { - n = f.ShortName() - } - if n == "name" { - // Avoid conflict with test struct's "name" field. - n = "n" - } else if n == "t" { - // Avoid conflict with test argument. - // "tr" is short for t receiver. - n = "tr" - } - return n -} - -func parameterName(f *models.Field) string { - var n string - if f.IsNamed() { - n = f.Name - } else { - n = fmt.Sprintf("in%v", f.Index) - } - return n -} - -func wantName(f *models.Field) string { - var n string - if f.IsNamed() { - n = "want" + strings.Title(f.Name) - } else if f.Index == 0 { - n = "want" - } else { - n = fmt.Sprintf("want%v", f.Index) - } - return n -} - -func gotName(f *models.Field) string { - var n string - if f.IsNamed() { - n = "got" + strings.Title(f.Name) - } else if f.Index == 0 { - n = "got" - } else { - n = fmt.Sprintf("got%v", f.Index) +func (r *Render) LoadFromData(templateData [][]byte) { + for _, d := range templateData { + r.tmpls = template.Must(r.tmpls.Parse(string(d))) } - return n } -func Header(w io.Writer, h *models.Header) error { - if err := tmpls.ExecuteTemplate(w, "header", h); err != nil { +func (r *Render) Header(w io.Writer, h *models.Header) error { + if err := r.tmpls.ExecuteTemplate(w, "header", h); err != nil { return err } _, err := w.Write(h.Code) return err } -func TestFunction(w io.Writer, f *models.Function, printInputs, subtests, named, parallel bool, templateParams map[string]interface{}) error { - return tmpls.ExecuteTemplate(w, "function", struct { +func (r *Render) TestFunction( + w io.Writer, + f *models.Function, + printInputs bool, + subtests bool, + named bool, + parallel bool, + params map[string]interface{}) error { + return r.tmpls.ExecuteTemplate(w, "function", struct { *models.Function PrintInputs bool Subtests bool @@ -181,6 +124,6 @@ func TestFunction(w io.Writer, f *models.Function, printInputs, subtests, named, Subtests: subtests, Parallel: parallel, Named: named, - TemplateParams: templateParams, + TemplateParams: params, }) } From 322e80c619629a253642d36f4e9ebe4c6e15748f Mon Sep 17 00:00:00 2001 From: Oleg Butuzov Date: Thu, 20 May 2021 17:45:09 +0300 Subject: [PATCH 2/3] docs: adds notes on failing tests (if run with `-race` flag) --- gotests_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gotests_test.go b/gotests_test.go index f29ece4..7b2f3d8 100644 --- a/gotests_test.go +++ b/gotests_test.go @@ -379,7 +379,6 @@ func TestGenerateTests(t *testing.T) { { name: "Receiver is indirect imported struct", args: args{ - // only: regexp.MustCompile("^Foo037$"), srcPath: `testdata/test037.go`, }, want: mustReadAndFormatGoFile(t, "testdata/goldens/receiver_is_indirect_imported_struct.go"), @@ -559,7 +558,7 @@ func TestGenerateTests(t *testing.T) { }, want: mustReadAndFormatGoFile(t, "testdata/goldens/existing_test_file_with_multiple_imports.go"), }, - { + { // WORNING: data race condition, if called with -race flag, because of structure in `internal/templates` package. name: "Entire testdata directory", args: args{ srcPath: `testdata/`, @@ -713,7 +712,7 @@ func TestGenerateTests(t *testing.T) { }, want: mustReadAndFormatGoFile(t, "testdata/goldens/function_with_return_value_custom_template.go"), }, - { + { // WORNING: panics on -race flag. name: "Test interface embedding", args: args{ srcPath: `testdata/undefinedtypes/interface_embedding.go`, From c76fad4c01748196e45edf9ca3149d6c9bab5669 Mon Sep 17 00:00:00 2001 From: Oleg Butuzov Date: Thu, 20 May 2021 17:46:21 +0300 Subject: [PATCH 3/3] fix: remove unused code & leave comments --- internal/render/helpers.go | 35 +---------------------------------- internal/render/render.go | 14 ++++---------- 2 files changed, 5 insertions(+), 44 deletions(-) diff --git a/internal/render/helpers.go b/internal/render/helpers.go index 02e00c7..20af67b 100755 --- a/internal/render/helpers.go +++ b/internal/render/helpers.go @@ -3,19 +3,12 @@ package render //go:generate esc -o bindata/esc.go -pkg=bindata templates import ( "fmt" - "io" "strings" - "text/template" "github.com/cweill/gotests/internal/models" ) -const ( - name = "name" - nFile = 7 -) - -var tmpls *template.Template +const nFile = 7 // Number of files to be read from template (package) template (directory) func fieldName(f *models.Field) string { var n string @@ -78,29 +71,3 @@ func gotName(f *models.Field) string { } return n } - -func Header(w io.Writer, h *models.Header) error { - if err := tmpls.ExecuteTemplate(w, "header", h); err != nil { - return err - } - _, err := w.Write(h.Code) - return err -} - -func TestFunction(w io.Writer, f *models.Function, printInputs, subtests, named, parallel bool, templateParams map[string]interface{}) error { - return tmpls.ExecuteTemplate(w, "function", struct { - *models.Function - PrintInputs bool - Subtests bool - Parallel bool - Named bool - TemplateParams map[string]interface{} - }{ - Function: f, - PrintInputs: printInputs, - Subtests: subtests, - Parallel: parallel, - Named: named, - TemplateParams: templateParams, - }) -} diff --git a/internal/render/render.go b/internal/render/render.go index cedcd9b..4317420 100755 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -29,10 +29,6 @@ func New() *Render { // default templates first for _, name := range bindata.AssetNames() { - // if strings.Contains(bindata.FSMustString(false, name), "function") { - // fmt.Printf("render.New - bindata.[%s]\n", name) - // // fmt.Printf("> %s\n", bindata.FSMustString(false, name)) - // } r.tmpls = template.Must(r.tmpls.Parse(bindata.FSMustString(false, name))) } @@ -50,9 +46,6 @@ func (r *Render) LoadCustomTemplates(dir string) error { for _, f := range files { templateFiles = append(templateFiles, path.Join(dir, f.Name())) } - - // fmt.Printf("template(s)- %#v\n", templateFiles) - r.tmpls, err = r.tmpls.ParseFiles(templateFiles...) if err != nil { return fmt.Errorf("tmpls.ParseFiles: %v", err) @@ -79,16 +72,17 @@ func (r *Render) LoadCustomTemplatesName(name string) error { return fmt.Errorf("templates.FSString: %v", err) } - tmpls, err = r.tmpls.Parse(text) - if err != nil { + if tmpls, err := r.tmpls.Parse(text); err != nil { return fmt.Errorf("tmpls.Parse: %v", err) + } else { + r.tmpls = tmpls } - r.tmpls = tmpls } return nil } +// LoadFromData allows to load from a data slice func (r *Render) LoadFromData(templateData [][]byte) { for _, d := range templateData { r.tmpls = template.Must(r.tmpls.Parse(string(d)))