Merge branch 'master' into coverage-tests-fix

.travis.yml

@@ -4,6 +4,7 @@ go:
   - 1.7.x
   - 1.8.x
   - 1.9.x
+  - 1.10.x
 
 install:
   - go get -v -t ./...

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## HEAD
 
 - When using custom reporters register the custom reporters *before* the default reporter.  This allows users to see the output of any print statements in their customer reporters. [#365]
+- When running a test and calculating the coverage using the `-coverprofile` and `-outputdir` flags, Ginkgo fails with an error if the directory does not exist. This is due to an [issue in go 1.10](https://github.com/golang/go/issues/24588) [#446]
 
 ## 1.4.0 7/16/2017
 

RELEASING.md

@@ -0,0 +1,8 @@
+A Ginkgo release is a tagged git sha and a GitHub release.  To cut a release:
+
+1. Ensure CHANGELOG.md is up to date.
+1. Update `VERSION` in `config/config.go`
+1. Create a commit with the version number as the commit message (e.g. `v1.3.0`)
+1. Tag the commit with the version number as the tag name (e.g. `v1.3.0`)
+1. Push the commit and tag to GitHub
+1. Create a new [GitHub release](https://help.github.com/articles/creating-releases/) with the version number as the tag  (e.g. `v1.3.0`).  List the key changes in the release notes.

ginkgo/run_command.go

@@ -104,22 +104,25 @@ func (r *SpecRunner) RunSpecs(args []string, additionalArgs []string) {
 		runResult, numSuites = r.suiteRunner.RunSuites(randomizedRunners, r.commandFlags.NumCompilers, r.commandFlags.KeepGoing, nil)
 	}
 
+	for _, runner := range runners {
+		runner.CleanUp()
+	}
+
 	if r.isInCoverageMode() {
 		if r.getOutputDir() != "" {
 			// If coverprofile is set, combine coverages
 			if r.getCoverprofile() != "" {
-				r.combineCoverprofiles(runners)
+				if err := r.combineCoverprofiles(runners); err != nil {
+					fmt.Println(err.Error())
+					os.Exit(1)
+				}
 			} else {
 				// Just move them
 				r.moveCoverprofiles(runners)
 			}
 		}
 	}
 
-	for _, runner := range runners {
-		runner.CleanUp()
-	}
-
 	fmt.Printf("\nGinkgo ran %d %s in %s\n", numSuites, pluralizedWord("suite", "suites", numSuites), time.Since(t))
 
 	if runResult.Passed {
@@ -151,38 +154,41 @@ func (r *SpecRunner) moveCoverprofiles(runners []*testrunner.TestRunner) {
 }
 
 // Combines all generated profiles in the specified directory
-func (r *SpecRunner) combineCoverprofiles(runners []*testrunner.TestRunner) {
+func (r *SpecRunner) combineCoverprofiles(runners []*testrunner.TestRunner) error {
 
 	path, _ := filepath.Abs(r.getOutputDir())
+	if !fileExists(path) {
+		return fmt.Errorf("Unable to create combined profile, outputdir does not exist: %s", r.getOutputDir())
+	}
 
 	fmt.Println("path is " + path)
-	os.MkdirAll(path, os.ModePerm)
 
 	combined, err := os.OpenFile(filepath.Join(path, r.getCoverprofile()),
 		os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0666)
 
 	if err != nil {
 		fmt.Printf("Unable to create combined profile, %v\n", err)
-		return
+		return nil // non-fatal error
 	}
 
 	for _, runner := range runners {
 		contents, err := ioutil.ReadFile(runner.CoverageFile)
 
 		if err != nil {
 			fmt.Printf("Unable to read coverage file %s to combine, %v\n", runner.CoverageFile, err)
-			return
+			return nil // non-fatal error
 		}
 
 		_, err = combined.Write(contents)
 
 		if err != nil {
 			fmt.Printf("Unable to append to coverprofile, %v\n", err)
-			return
+			return nil // non-fatal error
 		}
 	}
 
 	fmt.Println("All profiles combined")
+	return nil
 }
 
 func (r *SpecRunner) isInCoverageMode() bool {

ginkgo/unfocus_command.go

@@ -26,6 +26,8 @@ func unfocusSpecs([]string, []string) {
 	unfocus("Measure")
 	unfocus("DescribeTable")
 	unfocus("Entry")
+	unfocus("Specify")
+	unfocus("When")
 }
 
 func unfocus(component string) {

integration/_fixtures/coverage_fixture/coverage.go

@@ -1,5 +1,9 @@
 package coverage_fixture
 
+import (
+	_ "github.com/onsi/ginkgo/integration/_fixtures/coverage_fixture/external_coverage_fixture"
+)
+
 func A() string {
 	return "A"
 }

integration/_fixtures/flags_tests/flags_test.go

@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	remapped "math"
+	_ "math/cmplx"
 	"time"
 
 	. "github.com/onsi/ginkgo"

integration/_fixtures/focused_fixture/focused_fixture_test.go

@@ -18,10 +18,20 @@ var _ = Describe("FocusedFixture", func() {
 		})
 	})
 
+	FWhen("focused", func() {
+		It("focused", func() {
+
+		})
+	})
+
 	FIt("focused", func() {
 
 	})
 
+	FSpecify("focused", func() {
+
+	})
+
 	FMeasure("focused", func(b Benchmarker) {
 
 	}, 2)

integration/coverage_test.go

@@ -71,6 +71,7 @@ var _ = Describe("Coverage Specs", func() {
 
 	Context("when run in recursive mode", func() {
 		AfterEach(func() {
+			removeSuccessfully("./_fixtures/combined_coverage_fixture/coverage-recursive.txt")
 			removeSuccessfully("./_fixtures/combined_coverage_fixture/first_package/coverage-recursive.txt")
 			removeSuccessfully("./_fixtures/combined_coverage_fixture/second_package/coverage-recursive.txt")
 		})
@@ -120,19 +121,12 @@ var _ = Describe("Coverage Specs", func() {
 		})
 	})
 
-	Context("when run in recursive mode specifying both a coverprofile and an outputdir", func() {
-		AfterEach(func() {
-			removeSuccessfully("./_fixtures/combined_coverage_fixture/all")
-			removeSuccessfully("./_fixtures/combined_coverage_fixture/coverage.txt")
-			removeSuccessfully("./_fixtures/combined_coverage_fixture/first_package/coverage.txt")
-			removeSuccessfully("./_fixtures/combined_coverage_fixture/second_package/coverage.txt")
-		})
-		It("Creates directories in path if they don't exist", func() {
-			session := startGinkgo("./_fixtures/combined_coverage_fixture", "-outputdir=./all/profiles/here", "-r", "-cover", "-coverprofile=coverage.txt")
-			Eventually(session).Should(gexec.Exit(0))
+	It("Fails with an error if output dir and coverprofile were set, but the output dir did not exist", func() {
+		session := startGinkgo("./_fixtures/combined_coverage_fixture", "-outputdir=./all/profiles/here", "-r", "-cover", "-coverprofile=coverage.txt")
 
-			Ω("./_fixtures/combined_coverage_fixture/all/profiles/here/coverage.txt").Should(BeARegularFile())
-		})
+		Eventually(session).Should(gexec.Exit(1))
+		output := session.Out.Contents()
+		Ω(string(output)).Should(ContainSubstring("Unable to create combined profile, outputdir does not exist: ./all/profiles/here"))
 	})
 
 	Context("when only output dir was set", func() {

integration/flags_test.go

@@ -213,6 +213,7 @@ var _ = Describe("Flags Specs", func() {
 		output = regextest("-regexScansFilePath=false", "-focus=/passing/") // nothing gets focused (nothing runs)
 		Ω(output).Should(ContainSubstring("0 of 4 Specs"))
 	})
+
 	It("should honor compiler flags", func() {
 		session := startGinkgo(pathToTest, "-gcflags=-importmap 'math=math/cmplx'")
 		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))

integration/subcommand_test.go

@@ -354,17 +354,17 @@ var _ = Describe("Subcommand", func() {
 			Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
 			output := session.Out.Contents()
 
-			Ω(output).Should(ContainSubstring("6 Passed"))
-			Ω(output).Should(ContainSubstring("5 Skipped"))
+			Ω(string(output)).Should(ContainSubstring("8 Passed"))
+			Ω(string(output)).Should(ContainSubstring("5 Skipped"))
 
 			session = startGinkgo(pathToTest, "blur")
 			Eventually(session).Should(gexec.Exit(0))
 
 			session = startGinkgo(pathToTest, "--noColor")
 			Eventually(session).Should(gexec.Exit(0))
 			output = session.Out.Contents()
-			Ω(output).Should(ContainSubstring("11 Passed"))
-			Ω(output).Should(ContainSubstring("0 Skipped"))
+			Ω(string(output)).Should(ContainSubstring("13 Passed"))
+			Ω(string(output)).Should(ContainSubstring("0 Skipped"))
 		})
 	})
 

internal/spec/spec.go

@@ -5,6 +5,8 @@ import (
 	"io"
 	"time"
 
+	"sync"
+
 	"github.com/onsi/ginkgo/internal/containernode"
 	"github.com/onsi/ginkgo/internal/leafnodes"
 	"github.com/onsi/ginkgo/types"
@@ -22,6 +24,8 @@ type Spec struct {
 	startTime        time.Time
 	failure          types.SpecFailure
 	previousFailures bool
+
+	stateMutex *sync.Mutex
 }
 
 func New(subject leafnodes.SubjectNode, containers []*containernode.ContainerNode, announceProgress bool) *Spec {
@@ -30,6 +34,7 @@ func New(subject leafnodes.SubjectNode, containers []*containernode.ContainerNod
 		containers:       containers,
 		focused:          subject.Flag() == types.FlagTypeFocused,
 		announceProgress: announceProgress,
+		stateMutex:       &sync.Mutex{},
 	}
 
 	spec.processFlag(subject.Flag())
@@ -44,32 +49,32 @@ func (spec *Spec) processFlag(flag types.FlagType) {
 	if flag == types.FlagTypeFocused {
 		spec.focused = true
 	} else if flag == types.FlagTypePending {
-		spec.state = types.SpecStatePending
+		spec.setState(types.SpecStatePending)
 	}
 }
 
 func (spec *Spec) Skip() {
-	spec.state = types.SpecStateSkipped
+	spec.setState(types.SpecStateSkipped)
 }
 
 func (spec *Spec) Failed() bool {
-	return spec.state == types.SpecStateFailed || spec.state == types.SpecStatePanicked || spec.state == types.SpecStateTimedOut
+	return spec.getState() == types.SpecStateFailed || spec.getState() == types.SpecStatePanicked || spec.getState() == types.SpecStateTimedOut
 }
 
 func (spec *Spec) Passed() bool {
-	return spec.state == types.SpecStatePassed
+	return spec.getState() == types.SpecStatePassed
 }
 
 func (spec *Spec) Flaked() bool {
-	return spec.state == types.SpecStatePassed && spec.previousFailures
+	return spec.getState() == types.SpecStatePassed && spec.previousFailures
 }
 
 func (spec *Spec) Pending() bool {
-	return spec.state == types.SpecStatePending
+	return spec.getState() == types.SpecStatePending
 }
 
 func (spec *Spec) Skipped() bool {
-	return spec.state == types.SpecStateSkipped
+	return spec.getState() == types.SpecStateSkipped
 }
 
 func (spec *Spec) Focused() bool {
@@ -93,7 +98,7 @@ func (spec *Spec) Summary(suiteID string) *types.SpecSummary {
 	componentCodeLocations[len(spec.containers)] = spec.subject.CodeLocation()
 
 	runTime := spec.runTime
-	if runTime == 0 {
+	if runTime == 0 && !spec.startTime.IsZero() {
 		runTime = time.Since(spec.startTime)
 	}
 
@@ -102,7 +107,7 @@ func (spec *Spec) Summary(suiteID string) *types.SpecSummary {
 		NumberOfSamples:        spec.subject.Samples(),
 		ComponentTexts:         componentTexts,
 		ComponentCodeLocations: componentCodeLocations,
-		State:        spec.state,
+		State:        spec.getState(),
 		RunTime:      runTime,
 		Failure:      spec.failure,
 		Measurements: spec.measurementsReport(),
@@ -120,7 +125,7 @@ func (spec *Spec) ConcatenatedString() string {
 }
 
 func (spec *Spec) Run(writer io.Writer) {
-	if spec.state == types.SpecStateFailed {
+	if spec.getState() == types.SpecStateFailed {
 		spec.previousFailures = true
 	}
 
@@ -132,14 +137,26 @@ func (spec *Spec) Run(writer io.Writer) {
 	for sample := 0; sample < spec.subject.Samples(); sample++ {
 		spec.runSample(sample, writer)
 
-		if spec.state != types.SpecStatePassed {
+		if spec.getState() != types.SpecStatePassed {
 			return
 		}
 	}
 }
 
+func (spec *Spec) getState() types.SpecState {
+	spec.stateMutex.Lock()
+	defer spec.stateMutex.Unlock()
+	return spec.state
+}
+
+func (spec *Spec) setState(state types.SpecState) {
+	spec.stateMutex.Lock()
+	defer spec.stateMutex.Unlock()
+	spec.state = state
+}
+
 func (spec *Spec) runSample(sample int, writer io.Writer) {
-	spec.state = types.SpecStatePassed
+	spec.setState(types.SpecStatePassed)
 	spec.failure = types.SpecFailure{}
 	innerMostContainerIndexToUnwind := -1
 
@@ -149,8 +166,8 @@ func (spec *Spec) runSample(sample int, writer io.Writer) {
 			for _, afterEach := range container.SetupNodesOfType(types.SpecComponentTypeAfterEach) {
 				spec.announceSetupNode(writer, "AfterEach", container, afterEach)
 				afterEachState, afterEachFailure := afterEach.Run()
-				if afterEachState != types.SpecStatePassed && spec.state == types.SpecStatePassed {
-					spec.state = afterEachState
+				if afterEachState != types.SpecStatePassed && spec.getState() == types.SpecStatePassed {
+					spec.setState(afterEachState)
 					spec.failure = afterEachFailure
 				}
 			}
@@ -161,8 +178,10 @@ func (spec *Spec) runSample(sample int, writer io.Writer) {
 		innerMostContainerIndexToUnwind = i
 		for _, beforeEach := range container.SetupNodesOfType(types.SpecComponentTypeBeforeEach) {
 			spec.announceSetupNode(writer, "BeforeEach", container, beforeEach)
-			spec.state, spec.failure = beforeEach.Run()
-			if spec.state != types.SpecStatePassed {
+			s, f := beforeEach.Run()
+			spec.failure = f
+			spec.setState(s)
+			if spec.getState() != types.SpecStatePassed {
 				return
 			}
 		}
@@ -171,15 +190,19 @@ func (spec *Spec) runSample(sample int, writer io.Writer) {
 	for _, container := range spec.containers {
 		for _, justBeforeEach := range container.SetupNodesOfType(types.SpecComponentTypeJustBeforeEach) {
 			spec.announceSetupNode(writer, "JustBeforeEach", container, justBeforeEach)
-			spec.state, spec.failure = justBeforeEach.Run()
-			if spec.state != types.SpecStatePassed {
+			s, f := justBeforeEach.Run()
+			spec.failure = f
+			spec.setState(s)
+			if spec.getState() != types.SpecStatePassed {
 				return
 			}
 		}
 	}
 
 	spec.announceSubject(writer, spec.subject)
-	spec.state, spec.failure = spec.subject.Run()
+	s, f := spec.subject.Run()
+	spec.failure = f
+	spec.setState(s)
 }
 
 func (spec *Spec) announceSetupNode(writer io.Writer, nodeType string, container *containernode.ContainerNode, setupNode leafnodes.BasicNode) {

internal/specrunner/spec_runner_test.go

@@ -235,6 +235,13 @@ var _ = Describe("Spec Runner", func() {
 				Ω(reporter1.EndSummary.NumberOfPassedSpecs).Should(Equal(0))
 				Ω(reporter1.EndSummary.NumberOfFailedSpecs).Should(Equal(0))
 			})
+
+			It("should not report a slow test", func() {
+				summaries := reporter1.SpecSummaries
+				for _, s := range summaries {
+					Expect(s.RunTime).To(BeZero())
+				}
+			})
 		})
 	})