From 5bd9dce5d6893521bde7783e7db8ca9c88635307 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sat, 29 Sep 2018 00:10:55 +0200 Subject: [PATCH 01/10] lint + unused --- builder_go110.go | 29 +++++++++-------------------- builder_test.go | 2 +- cmd/godog/main.go | 19 ------------------- fmt.go | 13 +++++-------- fmt_cucumber.go | 17 ++++++++--------- suite.go | 6 ------ 6 files changed, 23 insertions(+), 63 deletions(-) diff --git a/builder_go110.go b/builder_go110.go index c4ae376..29bbbab 100644 --- a/builder_go110.go +++ b/builder_go110.go @@ -19,16 +19,15 @@ import ( "unicode" ) -var tooldir = findToolDir() -var compiler = filepath.Join(tooldir, "compile") -var linker = filepath.Join(tooldir, "link") -var gopaths = filepath.SplitList(build.Default.GOPATH) -var goarch = build.Default.GOARCH -var goroot = build.Default.GOROOT -var goos = build.Default.GOOS +var ( + tooldir = findToolDir() + compiler = filepath.Join(tooldir, "compile") + linker = filepath.Join(tooldir, "link") + gopaths = filepath.SplitList(build.Default.GOPATH) + godogImportPath = "github.com/DATA-DOG/godog" -var godogImportPath = "github.com/DATA-DOG/godog" -var runnerTemplate = template.Must(template.New("testmain").Parse(`package main + // godep + runnerTemplate = template.Must(template.New("testmain").Parse(`package main import ( "github.com/DATA-DOG/godog" @@ -45,6 +44,7 @@ func main() { }) os.Exit(status) }`)) +) // Build creates a test package like go test command at given target path. // If there are no go files in tested directory, then @@ -299,17 +299,6 @@ func makeImportValid(r rune) rune { return r } -func uniqStringList(strs []string) (unique []string) { - uniq := make(map[string]void, len(strs)) - for _, s := range strs { - if _, ok := uniq[s]; !ok { - uniq[s] = void{} - unique = append(unique, s) - } - } - return -} - // buildTestMain if given package is valid // it scans test files for contexts // and produces a testmain source code. diff --git a/builder_test.go b/builder_test.go index 4a1c5ac..820dd66 100644 --- a/builder_test.go +++ b/builder_test.go @@ -27,7 +27,7 @@ func TestBuildTestRunnerWithoutGoFiles(t *testing.T) { } defer func() { - os.Chdir(pwd) // get back to current dir + _ = os.Chdir(pwd) // get back to current dir }() if err := Build(bin); err != nil { diff --git a/cmd/godog/main.go b/cmd/godog/main.go index f409d07..5e1c97f 100644 --- a/cmd/godog/main.go +++ b/cmd/godog/main.go @@ -3,12 +3,10 @@ package main import ( "fmt" "go/build" - "io" "os" "os/exec" "path/filepath" "regexp" - "strconv" "syscall" "github.com/DATA-DOG/godog" @@ -107,20 +105,3 @@ func main() { } os.Exit(status) } - -func statusOutputFilter(w io.Writer) io.Writer { - return writerFunc(func(b []byte) (int, error) { - if m := statusMatch.FindStringSubmatch(string(b)); len(m) > 1 { - parsedStatus, _ = strconv.Atoi(m[1]) - // skip status stderr output - return len(b), nil - } - return w.Write(b) - }) -} - -type writerFunc func([]byte) (int, error) - -func (w writerFunc) Write(b []byte) (int, error) { - return w(b) -} diff --git a/fmt.go b/fmt.go index fdaf0ac..57d1297 100644 --- a/fmt.go +++ b/fmt.go @@ -381,9 +381,11 @@ func (f *basefmt) Summary() { } func (s *undefinedSnippet) Args() (ret string) { - var args []string - var pos, idx int - var breakLoop bool + var ( + args []string + pos int + breakLoop bool + ) for !breakLoop { part := s.Expr[pos:] ipos := strings.Index(part, "(\\d+)") @@ -392,25 +394,20 @@ func (s *undefinedSnippet) Args() (ret string) { case spos == -1 && ipos == -1: breakLoop = true case spos == -1: - idx++ pos += ipos + len("(\\d+)") args = append(args, reflect.Int.String()) case ipos == -1: - idx++ pos += spos + len("\"([^\"]*)\"") args = append(args, reflect.String.String()) case ipos < spos: - idx++ pos += ipos + len("(\\d+)") args = append(args, reflect.Int.String()) case spos < ipos: - idx++ pos += spos + len("\"([^\"]*)\"") args = append(args, reflect.String.String()) } } if s.argument != nil { - idx++ switch s.argument.(type) { case *gherkin.DocString: args = append(args, "*gherkin.DocString") diff --git a/fmt_cucumber.go b/fmt_cucumber.go index 19897ef..cbfe30d 100644 --- a/fmt_cucumber.go +++ b/fmt_cucumber.go @@ -109,15 +109,14 @@ type cukefmt struct { // this is sadly not passed by gherkin nodes. // it restricts this formatter to run only in synchronous single // threaded execution. Unless running a copy of formatter for each feature - path string - stat stepType // last step status, before skipped - outlineSteps int // number of current outline scenario steps - ID string // current test id. - results []cukeFeatureJSON // structure that represent cuke results - curStep *cukeStep // track the current step - curElement *cukeElement // track the current element - curFeature *cukeFeatureJSON // track the current feature - curOutline cukeElement // Each example show up as an outline element but the outline is parsed only once + path string + stat stepType // last step status, before skipped + ID string // current test id. + results []cukeFeatureJSON // structure that represent cuke results + curStep *cukeStep // track the current step + curElement *cukeElement // track the current element + curFeature *cukeFeatureJSON // track the current feature + curOutline cukeElement // Each example show up as an outline element but the outline is parsed only once // so I need to keep track of the current outline curRow int // current row of the example table as it is being processed. curExampleTags []cukeTag // temporary storage for tags associate with the current example table. diff --git a/suite.go b/suite.go index 643e172..7cf8619 100644 --- a/suite.go +++ b/suite.go @@ -402,12 +402,6 @@ func (s *Suite) runSteps(steps []*gherkin.Step) (err error) { return } -func (s *Suite) skipSteps(steps []*gherkin.Step) { - for _, step := range steps { - s.fmt.Skipped(step, s.matchStep(step)) - } -} - func (s *Suite) runOutline(outline *gherkin.ScenarioOutline, b *gherkin.Background) (failErr error) { s.fmt.Node(outline) From 6943e03b655e1d1dbcab4aff04fdbb5bbba746f9 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sat, 29 Sep 2018 00:14:45 +0200 Subject: [PATCH 02/10] lint + unused --- cmd/godog/main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/godog/main.go b/cmd/godog/main.go index 5e1c97f..2ac6adc 100644 --- a/cmd/godog/main.go +++ b/cmd/godog/main.go @@ -6,14 +6,12 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "syscall" "github.com/DATA-DOG/godog" "github.com/DATA-DOG/godog/colors" ) -var statusMatch = regexp.MustCompile("^exit status (\\d+)") var parsedStatus int func buildAndRun() (int, error) { From b03bcc9c558080949597bb49e5bd749ccfaa604e Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sat, 29 Sep 2018 12:28:54 +0200 Subject: [PATCH 03/10] deals with go modules and sub-packages --- builder_go110.go | 67 +++++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/builder_go110.go b/builder_go110.go index 29bbbab..fbd8af3 100644 --- a/builder_go110.go +++ b/builder_go110.go @@ -4,6 +4,7 @@ package godog import ( "bytes" + "encoding/json" "fmt" "go/build" "go/parser" @@ -303,45 +304,68 @@ func makeImportValid(r rune) rune { // it scans test files for contexts // and produces a testmain source code. func buildTestMain(pkg *build.Package) ([]byte, bool, error) { - var contexts []string - var importPath string - name := "main" + var ( + contexts []string + err error + name, importPath string + ) if nil != pkg { - ctxs, err := processPackageTestFiles( + contexts, err = processPackageTestFiles( pkg.TestGoFiles, pkg.XTestGoFiles, ) if err != nil { return nil, false, err } - contexts = ctxs - - // for module support, query the module import path - // @TODO: maybe there is a better way to read it - out, err := exec.Command("go", "list", "-m").CombinedOutput() - if err != nil { - // is not using modules or older go version - importPath = pkg.ImportPath - } else { - // otherwise read the module name from command output - importPath = strings.TrimSpace(string(out)) - } + importPath = parseImport(pkg.ImportPath) name = pkg.Name + } else { + name = "main" } - data := struct { Name string Contexts []string ImportPath string - }{name, contexts, importPath} - + }{ + Name: name, + Contexts: contexts, + ImportPath: importPath, + } var buf bytes.Buffer - if err := runnerTemplate.Execute(&buf, data); err != nil { + if err = runnerTemplate.Execute(&buf, data); err != nil { return nil, len(contexts) > 0, err } return buf.Bytes(), len(contexts) > 0, nil } +// parseImport parses the import path to deal with go module. +func parseImport(rawPath string) string { + // for module support, query the module import path + cmd := exec.Command("go", "list", "-m", "-json") + out, err := cmd.StdoutPipe() + if err != nil { + // Unable to read stdout + return rawPath + } + if cmd.Start() != nil { + // Does not using modules + return rawPath + } + var mod struct { + Dir string `json:"Dir"` + Path string `json:"Path"` + } + if json.NewDecoder(out).Decode(&mod) != nil { + // Unexpected result + return rawPath + } + if cmd.Wait() != nil { + return rawPath + } + // Concatenates the module path with the current sub-folders if needed + return mod.Path + filepath.ToSlash(strings.TrimPrefix(strings.TrimPrefix(rawPath, "_"), mod.Dir)) +} + // processPackageTestFiles runs through ast of each test // file pack and looks for godog suite contexts to register // on run @@ -389,16 +413,13 @@ func dependencies(pkg *build.Package, visited map[string]string, vendor bool) er if i := strings.LastIndex(name, "vendor/"); vendor && i == -1 { continue // only interested in vendor packages } - if _, ok := visited[name]; ok { continue } - next, err := locatePackage(name) if err != nil { return err } - visited[name] = pkg.PkgObj if err := dependencies(next, visited, vendor); err != nil { return err From 2604810f50ac5d34e5b279dea69f15b9248eef5a Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sat, 29 Sep 2018 13:12:16 +0200 Subject: [PATCH 04/10] errcheck + unused --- colors/colors.go | 4 ++-- colors/writer.go | 2 +- fmt_junit.go | 8 +++++--- fmt_junit_test.go | 13 ++++++------- run_test.go | 4 ++-- suite.go | 2 +- utils.go | 22 ++++++++++------------ 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/colors/colors.go b/colors/colors.go index df00cec..0232317 100644 --- a/colors/colors.go +++ b/colors/colors.go @@ -16,8 +16,8 @@ const ( red green yellow - blue - magenta + blue // unused + magenta // unused cyan white ) diff --git a/colors/writer.go b/colors/writer.go index 0d33659..469c7a5 100644 --- a/colors/writer.go +++ b/colors/writer.go @@ -16,7 +16,7 @@ type outputMode int const ( _ outputMode = iota discardNonColorEscSeq - outputNonColorEscSeq + outputNonColorEscSeq // unused ) // Colored creates and initializes a new ansiColorWriter diff --git a/fmt_junit.go b/fmt_junit.go index 8762cbe..0a0f27b 100644 --- a/fmt_junit.go +++ b/fmt_junit.go @@ -148,11 +148,13 @@ func (j *junitFormatter) Summary() { j.current().Time = timeNowFunc().Sub(j.featStarted).String() } j.suite.Time = timeNowFunc().Sub(j.started).String() - io.WriteString(j.out, xml.Header) - + _, err := io.WriteString(j.out, xml.Header) + if err != nil { + fmt.Fprintln(os.Stderr, "failed to write junit string:", err) + } enc := xml.NewEncoder(j.out) enc.Indent("", s(2)) - if err := enc.Encode(j.suite); err != nil { + if err = enc.Encode(j.suite); err != nil { fmt.Fprintln(os.Stderr, "failed to write junit xml:", err) } } diff --git a/fmt_junit_test.go b/fmt_junit_test.go index 0a5fa5c..36f6257 100644 --- a/fmt_junit_test.go +++ b/fmt_junit_test.go @@ -152,19 +152,18 @@ func TestJUnitFormatterOutput(t *testing.T) { }, }}, } - s.run() s.fmt.Summary() var exp bytes.Buffer - io.WriteString(&exp, xml.Header) - - enc := xml.NewEncoder(&exp) - enc.Indent("", " ") - if err := enc.Encode(expected); err != nil { + if _, err = io.WriteString(&exp, xml.Header); err != nil { + t.Fatalf("unexpected error: %v", err) + } + enc := xml.NewEncoder(&exp) + enc.Indent("", " ") + if err = enc.Encode(expected); err != nil { t.Fatalf("unexpected error: %v", err) } - if buf.String() != exp.String() { t.Fatalf("expected output does not match: %s", buf.String()) } diff --git a/run_test.go b/run_test.go index 8cb5032..855a5e5 100644 --- a/run_test.go +++ b/run_test.go @@ -178,8 +178,8 @@ func TestFailsWithUnknownFormatterOptionError(t *testing.T) { } out := strings.TrimSpace(string(b)) - if strings.Index(out, `unregistered formatter name: "unknown", use one of`) == -1 { - t.Fatalf("unexpected error output: \"%s\"", out) + if !strings.Contains(out, `unregistered formatter name: "unknown", use one of`) { + t.Fatalf("unexpected error output: %q", out) } } diff --git a/suite.go b/suite.go index 7cf8619..d262a40 100644 --- a/suite.go +++ b/suite.go @@ -805,7 +805,7 @@ func matchesTags(filter string, tags []string) (ok bool) { okComma = hasTag(tags, tag) || okComma } } - ok = (false != okComma && ok && okComma) || false + ok = ok && okComma } return } diff --git a/utils.go b/utils.go index 3fb1588..fe824a9 100644 --- a/utils.go +++ b/utils.go @@ -7,18 +7,16 @@ import ( "github.com/DATA-DOG/godog/colors" ) -// empty struct value takes no space allocation -type void struct{} - -var red = colors.Red -var redb = colors.Bold(colors.Red) -var green = colors.Green -var black = colors.Black -var blackb = colors.Bold(colors.Black) -var yellow = colors.Yellow -var cyan = colors.Cyan -var cyanb = colors.Bold(colors.Cyan) -var whiteb = colors.Bold(colors.White) +var ( + red = colors.Red + redb = colors.Bold(colors.Red) + green = colors.Green + black = colors.Black + yellow = colors.Yellow + cyan = colors.Cyan + cyanb = colors.Bold(colors.Cyan) + whiteb = colors.Bold(colors.White) +) // repeats a space n times func s(n int) string { From 807284d0160da0e9bde180b7187ee1eda31877d6 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sat, 29 Sep 2018 18:29:40 +0200 Subject: [PATCH 05/10] void type is used to build the go code < 1.10 --- utils.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils.go b/utils.go index fe824a9..34b9683 100644 --- a/utils.go +++ b/utils.go @@ -7,6 +7,9 @@ import ( "github.com/DATA-DOG/godog/colors" ) +// build < go.10 +type void struct{} + var ( red = colors.Red redb = colors.Bold(colors.Red) From dbdf7a7ebb27d6e48192ee743f0ca74cb7e5ad87 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sun, 30 Sep 2018 00:44:47 +0200 Subject: [PATCH 06/10] fmt failed --- fmt.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fmt.go b/fmt.go index 57d1297..476708c 100644 --- a/fmt.go +++ b/fmt.go @@ -382,8 +382,8 @@ func (f *basefmt) Summary() { func (s *undefinedSnippet) Args() (ret string) { var ( - args []string - pos int + args []string + pos int breakLoop bool ) for !breakLoop { From 4b953c6733f03f36e4b814253f76ee9e5c597702 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sun, 30 Sep 2018 13:42:48 +0200 Subject: [PATCH 07/10] unconvert --- fmt_progress.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fmt_progress.go b/fmt_progress.go index a63e474..dbbf32c 100644 --- a/fmt_progress.go +++ b/fmt_progress.go @@ -47,7 +47,7 @@ func (f *progress) Feature(ft *gherkin.Feature, p string, c []byte) { func (f *progress) Summary() { left := math.Mod(float64(f.steps), float64(f.stepsPerRow)) if left != 0 { - if int(f.steps) > f.stepsPerRow { + if f.steps > f.stepsPerRow { fmt.Fprintf(f.out, s(f.stepsPerRow-int(left))+fmt.Sprintf(" %d\n", f.steps)) } else { fmt.Fprintf(f.out, " %d\n", f.steps) From 8c3f57ba1611f4fac00b5da8da2299f44dd7173d Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sun, 30 Sep 2018 14:21:02 +0200 Subject: [PATCH 08/10] move void type where it is used (build < go.1.10) --- builder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builder.go b/builder.go index d27d25f..b19dfb7 100644 --- a/builder.go +++ b/builder.go @@ -241,6 +241,8 @@ func makeImportValid(r rune) rune { return r } +type void struct{} + func uniqStringList(strs []string) (unique []string) { uniq := make(map[string]void, len(strs)) for _, s := range strs { From 479842c91e107e0558e8e8b7ccffbd5953fc430a Mon Sep 17 00:00:00 2001 From: hgouchet Date: Sun, 30 Sep 2018 14:23:03 +0200 Subject: [PATCH 09/10] move void type where it is used (build < go.1.10) --- utils.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/utils.go b/utils.go index 34b9683..fe824a9 100644 --- a/utils.go +++ b/utils.go @@ -7,9 +7,6 @@ import ( "github.com/DATA-DOG/godog/colors" ) -// build < go.10 -type void struct{} - var ( red = colors.Red redb = colors.Bold(colors.Red) From f92dc4c9b7ae91910825f7e26b317b901e905f14 Mon Sep 17 00:00:00 2001 From: hgouchet Date: Tue, 2 Oct 2018 11:03:30 +0200 Subject: [PATCH 10/10] deal with modules inside the gopath --- builder_go110.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/builder_go110.go b/builder_go110.go index fbd8af3..9635102 100644 --- a/builder_go110.go +++ b/builder_go110.go @@ -317,7 +317,7 @@ func buildTestMain(pkg *build.Package) ([]byte, bool, error) { if err != nil { return nil, false, err } - importPath = parseImport(pkg.ImportPath) + importPath = parseImport(pkg.ImportPath, pkg.Root) name = pkg.Name } else { name = "main" @@ -339,7 +339,13 @@ func buildTestMain(pkg *build.Package) ([]byte, bool, error) { } // parseImport parses the import path to deal with go module. -func parseImport(rawPath string) string { +func parseImport(rawPath, rootPath string) string { + // with go > 1.11 and go module enabled out of the GOPATH, + // the import path begins with an underscore and the GOPATH is unknown on build. + if rootPath != "" { + // go < 1.11 or it's a module inside the GOPATH + return rawPath + } // for module support, query the module import path cmd := exec.Command("go", "list", "-m", "-json") out, err := cmd.StdoutPipe()