Merge pull request #275 from cucumber/concurrent-pretty-formatter

Added concurrency support to the pretty formatter

fmt.go

@@ -68,6 +68,7 @@ func AvailableFormatters() map[string]string {
 // formatters needs to be registered with a
 // godog.Format function call
 type Formatter interface {
+	TestRunStarted()
 	Feature(*messages.GherkinDocument, string, []byte)
 	Pickle(*messages.Pickle)
 	Defined(*messages.Pickle, *messages.Pickle_PickleStep, *StepDefinition)
@@ -165,7 +166,8 @@ type basefmt struct {
 	started  time.Time
 	features []*feature
 
-	lock *sync.Mutex
+	firstFeature *bool
+	lock         *sync.Mutex
 }
 
 func (f *basefmt) lastFeature() *feature {
@@ -226,6 +228,14 @@ func (f *basefmt) findStep(stepAstID string) *messages.GherkinDocument_Feature_S
 	panic("Couldn't find step for AST ID: " + stepAstID)
 }
 
+func (f *basefmt) TestRunStarted() {
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
+	firstFeature := true
+	f.firstFeature = &firstFeature
+}
+
 func (f *basefmt) Pickle(p *messages.Pickle) {
 	f.lock.Lock()
 	defer f.lock.Unlock()
@@ -240,6 +250,8 @@ func (f *basefmt) Feature(ft *messages.GherkinDocument, p string, c []byte) {
 	f.lock.Lock()
 	defer f.lock.Unlock()
 
+	*f.firstFeature = false
+
 	f.features = append(f.features, &feature{Path: p, GherkinDocument: ft, time: timeNowFunc()})
 }
 
@@ -397,6 +409,7 @@ func (f *basefmt) Summary() {
 func (f *basefmt) Sync(cf ConcurrentFormatter) {
 	if source, ok := cf.(*basefmt); ok {
 		f.lock = source.lock
+		f.firstFeature = source.firstFeature
 	}
 }
 

fmt_junit_test.go

@@ -159,6 +159,7 @@ func TestJUnitFormatterOutput(t *testing.T) {
 		}},
 	}
 
+	s.fmt.TestRunStarted()
 	s.run()
 	s.fmt.Summary()
 

fmt_pretty.go

@@ -28,14 +28,27 @@ type pretty struct {
 }
 
 func (f *pretty) Feature(gd *messages.GherkinDocument, p string, c []byte) {
+	f.lock.Lock()
+	if !*f.firstFeature {
+		fmt.Fprintln(f.out, "")
+	}
+	f.lock.Unlock()
+
 	f.basefmt.Feature(gd, p, c)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printFeature(gd.Feature)
 }
 
 // Pickle takes a gherkin node for formatting
 func (f *pretty) Pickle(pickle *messages.Pickle) {
 	f.basefmt.Pickle(pickle)
 
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	if len(pickle.Steps) == 0 {
 		f.printUndefinedPickle(pickle)
 		return
@@ -44,34 +57,62 @@ func (f *pretty) Pickle(pickle *messages.Pickle) {
 
 func (f *pretty) Passed(pickle *messages.Pickle, step *messages.Pickle_PickleStep, match *StepDefinition) {
 	f.basefmt.Passed(pickle, step, match)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printStep(f.lastStepResult())
 }
 
 func (f *pretty) Skipped(pickle *messages.Pickle, step *messages.Pickle_PickleStep, match *StepDefinition) {
 	f.basefmt.Skipped(pickle, step, match)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printStep(f.lastStepResult())
 }
 
 func (f *pretty) Undefined(pickle *messages.Pickle, step *messages.Pickle_PickleStep, match *StepDefinition) {
 	f.basefmt.Undefined(pickle, step, match)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printStep(f.lastStepResult())
 }
 
 func (f *pretty) Failed(pickle *messages.Pickle, step *messages.Pickle_PickleStep, match *StepDefinition, err error) {
 	f.basefmt.Failed(pickle, step, match, err)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printStep(f.lastStepResult())
 }
 
 func (f *pretty) Pending(pickle *messages.Pickle, step *messages.Pickle_PickleStep, match *StepDefinition) {
 	f.basefmt.Pending(pickle, step, match)
+
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
 	f.printStep(f.lastStepResult())
 }
 
-func (f *pretty) printFeature(feature *messages.GherkinDocument_Feature) {
-	if len(f.features) > 1 {
-		fmt.Fprintln(f.out, "") // not a first feature, add a newline
+func (f *pretty) Sync(cf ConcurrentFormatter) {
+	if source, ok := cf.(*pretty); ok {
+		f.basefmt.Sync(source.basefmt)
 	}
+}
 
+func (f *pretty) Copy(cf ConcurrentFormatter) {
+	if source, ok := cf.(*pretty); ok {
+		f.basefmt.Copy(source.basefmt)
+	}
+}
+
+func (f *pretty) printFeature(feature *messages.GherkinDocument_Feature) {
 	fmt.Fprintln(f.out, keywordAndName(feature.Keyword, feature.Name))
 	if strings.TrimSpace(feature.Description) != "" {
 		for _, line := range strings.Split(feature.Description, "\n") {

formatters_print_test.go

@@ -47,12 +47,12 @@ func TestPrintingFormatters(t *testing.T) {
 			expectedOutput, err := ioutil.ReadFile(expectOutputPath)
 			require.NoError(t, err)
 
+			suite.fmt.TestRunStarted()
 			suite.run()
 			suite.fmt.Summary()
 
 			expected := string(expectedOutput)
 			actual := buf.String()
-
 			assert.Equalf(t, expected, actual, "path: %s", expectOutputPath)
 		}
 	}

run.go

@@ -38,6 +38,8 @@ func (r *runner) concurrent(rate int, formatterFn func() Formatter) (failed bool
 		useFmtCopy = true
 	}
 
+	r.fmt.TestRunStarted()
+
 	queue := make(chan int, rate)
 	for i, ft := range r.features {
 		queue <- i // reserve space in queue
@@ -117,9 +119,11 @@ func (r *runner) run() bool {
 		features:      r.features,
 	}
 	r.initializer(suite)
-	suite.run()
 
+	r.fmt.TestRunStarted()
+	suite.run()
 	r.fmt.Summary()
+
 	return suite.failed
 }
 
@@ -281,7 +285,7 @@ func supportsConcurrency(format string) bool {
 	case "cucumber":
 		return false
 	case "pretty":
-		return false
+		return true
 	default:
 		return true // enables concurrent custom formatters to work
 	}

run_test.go

@@ -124,30 +124,6 @@ func TestShouldFailOnError(t *testing.T) {
 	assert.True(t, r.run())
 }
 
-func TestFailsWithConcurrencyOptionError(t *testing.T) {
-	stderr, closer := bufErrorPipe(t)
-	defer closer()
-	defer stderr.Close()
-
-	opt := Options{
-		Format:      "pretty",
-		Paths:       []string{"features/load:6"},
-		Concurrency: 2,
-		Output:      ioutil.Discard,
-	}
-
-	status := RunWithOptions("fails", func(_ *Suite) {}, opt)
-	require.Equal(t, exitOptionError, status)
-
-	closer()
-
-	b, err := ioutil.ReadAll(stderr)
-	require.NoError(t, err)
-
-	out := strings.TrimSpace(string(b))
-	assert.Equal(t, `format "pretty" does not support concurrent execution`, out)
-}
-
 func TestFailsWithUnknownFormatterOptionError(t *testing.T) {
 	stderr, closer := bufErrorPipe(t)
 	defer closer()
@@ -275,6 +251,7 @@ func TestFormatterConcurrencyRun(t *testing.T) {
 	formatters := []string{
 		"progress",
 		"junit",
+		"pretty",
 	}
 
 	featurePaths := []string{"formatter-tests/features"}

suite_context.go

@@ -181,8 +181,11 @@ func (s *suiteContext) iRunFeatureSuiteWithTags(tags string) error {
 		applyTagFilter(tags, feat)
 	}
 	s.testedSuite.fmt = testFormatterFunc("godog", &s.out)
+
+	s.testedSuite.fmt.TestRunStarted()
 	s.testedSuite.run()
 	s.testedSuite.fmt.Summary()
+
 	return nil
 }
 
@@ -195,8 +198,11 @@ func (s *suiteContext) iRunFeatureSuiteWithFormatter(name string) error {
 	if err := s.parseFeatures(); err != nil {
 		return err
 	}
+
+	s.testedSuite.fmt.TestRunStarted()
 	s.testedSuite.run()
 	s.testedSuite.fmt.Summary()
+
 	return nil
 }
 
@@ -444,6 +450,8 @@ func (s *suiteContext) iRunFeatureSuite() error {
 		return err
 	}
 	s.testedSuite.fmt = testFormatterFunc("godog", &s.out)
+
+	s.testedSuite.fmt.TestRunStarted()
 	s.testedSuite.run()
 	s.testedSuite.fmt.Summary()