From b730b9fd4b9d7b08f7701814eb9ea1c29151bceb Mon Sep 17 00:00:00 2001 From: funvit Date: Sat, 31 Aug 2019 17:40:34 +0300 Subject: [PATCH 1/3] fix for progress formatter summary in concurrency mode see: https://github.com/DATA-DOG/godog/issues/161 --- run.go | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/run.go b/run.go index 84fbf34..e4ec538 100644 --- a/run.go +++ b/run.go @@ -8,6 +8,7 @@ import ( "runtime" "strconv" "strings" + "sync" "github.com/DATA-DOG/godog/colors" ) @@ -28,11 +29,22 @@ type runner struct { initializer initializer } -func (r *runner) concurrent(rate int) (failed bool) { +func (r *runner) concurrent(rate int, formatterFn func() Formatter) (failed bool) { + var useFmtCopy bool + var copyLock sync.Mutex + + // special mode for progress-formatter + if _, ok := r.fmt.(*progress); ok { + useFmtCopy = true + } + queue := make(chan int, rate) for i, ft := range r.features { queue <- i // reserve space in queue + ft := *ft + go func(fail *bool, feat *feature) { + var fmtCopy Formatter defer func() { <-queue // free a space in queue }() @@ -46,12 +58,48 @@ func (r *runner) concurrent(rate int) (failed bool) { strict: r.strict, features: []*feature{feat}, } + if useFmtCopy { + fmtCopy = formatterFn() + suite.fmt = fmtCopy + } else { + suite.fmt = r.fmt + } + r.initializer(suite) suite.run() if suite.failed { *fail = true } - }(&failed, ft) + if useFmtCopy { + copyLock.Lock() + if d, ok := r.fmt.(*progress); ok { + if s, ok := fmtCopy.(*progress); ok { + func(source *progress, dest *progress) { + for _, v := range source.features { + dest.features = append(dest.features, v) + } + + for _, v := range source.failed { + dest.failed = append(dest.failed, v) + } + for _, v := range source.passed { + dest.passed = append(dest.passed, v) + } + for _, v := range source.skipped { + dest.skipped = append(dest.skipped, v) + } + for _, v := range source.undefined { + dest.undefined = append(dest.undefined, v) + } + for _, v := range source.pending { + dest.pending = append(dest.pending, v) + } + }(s, d) + } + } + copyLock.Unlock() + } + }(&failed, &ft) } // wait until last are processed for i := 0; i < rate; i++ { @@ -168,7 +216,7 @@ func RunWithOptions(suite string, contextInitializer func(suite *Suite), opt Opt var failed bool if opt.Concurrency > 1 { - failed = r.concurrent(opt.Concurrency) + failed = r.concurrent(opt.Concurrency, func() Formatter { return formatter(suite, output) }) } else { failed = r.run() } From 0d22f34c56d8b98dc338b9a733e2a53c3dae5a4f Mon Sep 17 00:00:00 2001 From: funvit Date: Sat, 31 Aug 2019 19:26:57 +0300 Subject: [PATCH 2/3] fix: added panic if type assertions failed --- run.go | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/run.go b/run.go index e4ec538..d5c243c 100644 --- a/run.go +++ b/run.go @@ -72,30 +72,33 @@ func (r *runner) concurrent(rate int, formatterFn func() Formatter) (failed bool } if useFmtCopy { copyLock.Lock() - if d, ok := r.fmt.(*progress); ok { - if s, ok := fmtCopy.(*progress); ok { - func(source *progress, dest *progress) { - for _, v := range source.features { - dest.features = append(dest.features, v) - } + dest, dOk := r.fmt.(*progress) + source, sOk := fmtCopy.(*progress) - for _, v := range source.failed { - dest.failed = append(dest.failed, v) - } - for _, v := range source.passed { - dest.passed = append(dest.passed, v) - } - for _, v := range source.skipped { - dest.skipped = append(dest.skipped, v) - } - for _, v := range source.undefined { - dest.undefined = append(dest.undefined, v) - } - for _, v := range source.pending { - dest.pending = append(dest.pending, v) - } - }(s, d) + if dOk && sOk { + for _, v := range source.features { + dest.features = append(dest.features, v) } + + for _, v := range source.failed { + dest.failed = append(dest.failed, v) + } + for _, v := range source.passed { + dest.passed = append(dest.passed, v) + } + for _, v := range source.skipped { + dest.skipped = append(dest.skipped, v) + } + for _, v := range source.undefined { + dest.undefined = append(dest.undefined, v) + } + for _, v := range source.pending { + dest.pending = append(dest.pending, v) + } + } else if !dOk { + panic("cant cast dest formatter to progress-typed") + } else if !sOk { + panic("cant cast source formatter to progress-typed") } copyLock.Unlock() } From 652eedf03a1c4fb9df5e6de14b4f38af5d16c335 Mon Sep 17 00:00:00 2001 From: funvit Date: Mon, 2 Sep 2019 21:58:26 +0300 Subject: [PATCH 3/3] fix for progress printing with concurrency mode --- fmt_progress.go | 49 ++++++++++++++++++++++++++----------------------- run.go | 8 ++++++++ run_test.go | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/fmt_progress.go b/fmt_progress.go index 7003de0..52e338f 100644 --- a/fmt_progress.go +++ b/fmt_progress.go @@ -15,6 +15,7 @@ func init() { } func progressFunc(suite string, out io.Writer) Formatter { + steps := 0 return &progress{ basefmt: basefmt{ started: timeNowFunc(), @@ -22,35 +23,37 @@ func progressFunc(suite string, out io.Writer) Formatter { out: out, }, stepsPerRow: 70, + lock: new(sync.Mutex), + steps: &steps, } } type progress struct { basefmt - sync.Mutex + lock *sync.Mutex stepsPerRow int - steps int + steps *int } func (f *progress) Node(n interface{}) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Node(n) } func (f *progress) Feature(ft *gherkin.Feature, p string, c []byte) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Feature(ft, p, c) } func (f *progress) Summary() { - left := math.Mod(float64(f.steps), float64(f.stepsPerRow)) + left := math.Mod(float64(*f.steps), float64(f.stepsPerRow)) if left != 0 { - if f.steps > f.stepsPerRow { - fmt.Fprintf(f.out, s(f.stepsPerRow-int(left))+fmt.Sprintf(" %d\n", f.steps)) + 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) + fmt.Fprintf(f.out, " %d\n", *f.steps) } } fmt.Fprintln(f.out, "") @@ -79,43 +82,43 @@ func (f *progress) step(res *stepResult) { case pending: fmt.Fprint(f.out, yellow("P")) } - f.steps++ - if math.Mod(float64(f.steps), float64(f.stepsPerRow)) == 0 { - fmt.Fprintf(f.out, " %d\n", f.steps) + *f.steps++ + if math.Mod(float64(*f.steps), float64(f.stepsPerRow)) == 0 { + fmt.Fprintf(f.out, " %d\n", *f.steps) } } func (f *progress) Passed(step *gherkin.Step, match *StepDef) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Passed(step, match) f.step(f.passed[len(f.passed)-1]) } func (f *progress) Skipped(step *gherkin.Step, match *StepDef) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Skipped(step, match) f.step(f.skipped[len(f.skipped)-1]) } func (f *progress) Undefined(step *gherkin.Step, match *StepDef) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Undefined(step, match) f.step(f.undefined[len(f.undefined)-1]) } func (f *progress) Failed(step *gherkin.Step, match *StepDef, err error) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Failed(step, match, err) f.step(f.failed[len(f.failed)-1]) } func (f *progress) Pending(step *gherkin.Step, match *StepDef) { - f.Lock() - defer f.Unlock() + f.lock.Lock() + defer f.lock.Unlock() f.basefmt.Pending(step, match) f.step(f.pending[len(f.pending)-1]) } diff --git a/run.go b/run.go index d5c243c..3ad0640 100644 --- a/run.go +++ b/run.go @@ -61,6 +61,14 @@ func (r *runner) concurrent(rate int, formatterFn func() Formatter) (failed bool if useFmtCopy { fmtCopy = formatterFn() suite.fmt = fmtCopy + + // sync lock and steps for progress printing + if sf, ok := suite.fmt.(*progress); ok { + if rf, ok := r.fmt.(*progress); ok { + sf.lock = rf.lock + sf.steps = rf.steps + } + } } else { suite.fmt = r.fmt } diff --git a/run_test.go b/run_test.go index 751d08e..c79aff9 100644 --- a/run_test.go +++ b/run_test.go @@ -275,3 +275,40 @@ func TestFeatureFilePathParser(t *testing.T) { } } } + +func TestSucceedWithConcurrencyOption(t *testing.T) { + output := new(bytes.Buffer) + + opt := Options{ + Format: "progress", + NoColors: true, + Paths: []string{"features"}, + Concurrency: 2, + Output: output, + } + + expectedOutput := `...................................................................... 70 +...................................................................... 140 +...................................................................... 210 +....................................... 249 + + +60 scenarios (60 passed) +249 steps (249 passed) +0s` + + status := RunWithOptions("succeed", func(s *Suite) { SuiteContext(s) }, opt) + if status != exitSuccess { + t.Fatalf("expected exit status to be 0, but was: %d", status) + } + + b, err := ioutil.ReadAll(output) + if err != nil { + t.Fatal(err) + } + + out := strings.TrimSpace(string(b)) + if out != expectedOutput { + t.Fatalf("unexpected output: \"%s\"", out) + } +}