Skip to content

Commit 2d5075a

Browse files
onsionsi
committedFeb 13, 2023
lock around default report output to avoid triggering the race detector when calling By from goroutines
Fixes #1134
·
v2.23.4v2.8.1
1 parent febbe38 commit 2d5075a

File tree

4 files changed

+63
-29
lines changed

4 files changed

+63
-29
lines changed
 

‎integration/_fixtures/passing_ginkgo_tests_fixture/passing_ginkgo_tests_test.go

+17
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package passing_ginkgo_tests_test
22

33
import (
4+
"sync"
5+
46
. "github.com/onsi/ginkgo/v2"
57
. "github.com/onsi/ginkgo/v2/integration/_fixtures/passing_ginkgo_tests_fixture"
68
. "github.com/onsi/gomega"
@@ -27,4 +29,19 @@ var _ = Describe("PassingGinkgoTests", func() {
2729
By("emitting another By")
2830
Ω(4).Should(Equal(4))
2931
})
32+
33+
Context("when called within goroutines", func() {
34+
It("does not trigger the race detector", func() {
35+
wg := &sync.WaitGroup{}
36+
wg.Add(3)
37+
for i := 0; i < 3; i += 1 {
38+
go func() {
39+
By("avoiding the race detector")
40+
wg.Done()
41+
}()
42+
}
43+
wg.Wait()
44+
})
45+
})
46+
3047
})

‎integration/run_test.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ var _ = Describe("Running Specs", func() {
3434

3535
Ω(output).Should(ContainSubstring("Running Suite: Passing_ginkgo_tests Suite"))
3636
Ω(output).Should(ContainSubstring(strings.Repeat(denoter, 4)))
37-
Ω(output).Should(ContainSubstring("SUCCESS! -- 4 Passed"))
37+
Ω(output).Should(ContainSubstring("SUCCESS! -- 5 Passed"))
3838
Ω(output).Should(ContainSubstring("Test Suite Passed"))
3939
})
4040
})
@@ -51,7 +51,7 @@ var _ = Describe("Running Specs", func() {
5151

5252
Ω(output).Should(ContainSubstring("Running Suite: Passing_ginkgo_tests Suite"))
5353
Ω(output).Should(ContainSubstring(strings.Repeat(denoter, 4)))
54-
Ω(output).Should(ContainSubstring("SUCCESS! -- 4 Passed"))
54+
Ω(output).Should(ContainSubstring("SUCCESS! -- 5 Passed"))
5555
Ω(output).Should(ContainSubstring("Test Suite Passed"))
5656
})
5757
})
@@ -268,7 +268,7 @@ var _ = Describe("Running Specs", func() {
268268
Eventually(session).Should(gexec.Exit(0))
269269
output := string(session.Out.Contents())
270270

271-
Ω(output).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs - 2 procs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s`, regexp.QuoteMeta(denoter)))
271+
Ω(output).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs - 2 procs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s`, regexp.QuoteMeta(denoter)))
272272
Ω(output).Should(ContainSubstring("Test Suite Passed"))
273273
})
274274
})
@@ -286,7 +286,7 @@ var _ = Describe("Running Specs", func() {
286286
if nodes > 4 {
287287
nodes = nodes - 1
288288
}
289-
Ω(output).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs - %d procs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]?s`, nodes, regexp.QuoteMeta(denoter)))
289+
Ω(output).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs - %d procs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]?s`, nodes, regexp.QuoteMeta(denoter)))
290290
Ω(output).Should(ContainSubstring("Test Suite Passed"))
291291
})
292292
})
@@ -341,7 +341,7 @@ var _ = Describe("Running Specs", func() {
341341
outputLines := strings.Split(output, "\n")
342342
Ω(outputLines[0]).Should(MatchRegexp(`\[\d+\] More_ginkgo_tests Suite - 2/2 specs [%s]{2} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
343343
Ω(outputLines[1]).Should(ContainSubstring("Skipping ./no_tagged_tests (no test files)"))
344-
Ω(outputLines[2]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
344+
Ω(outputLines[2]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
345345
Ω(output).Should(ContainSubstring("Test Suite Passed"))
346346
})
347347
})
@@ -354,7 +354,7 @@ var _ = Describe("Running Specs", func() {
354354
outputLines := strings.Split(output, "\n")
355355
Ω(outputLines[0]).Should(MatchRegexp(`\[\d+\] More_ginkgo_tests Suite - 2/2 specs [%s]{2} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
356356
Ω(outputLines[1]).Should(ContainSubstring("Skipping ./no_tagged_tests (no test files)"))
357-
Ω(outputLines[2]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
357+
Ω(outputLines[2]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
358358
Ω(output).Should(ContainSubstring("Test Suite Passed"))
359359
})
360360
})
@@ -372,7 +372,7 @@ var _ = Describe("Running Specs", func() {
372372

373373
outputLines := strings.Split(output, "\n")
374374
Ω(outputLines[0]).Should(ContainSubstring("Skipping ./no_tagged_tests (no test files)"))
375-
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
375+
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
376376
Ω(outputLines[2]).Should(MatchRegexp(`\[\d+\] Failing_ginkgo_tests Suite - 2/2 specs`))
377377
Ω(output).Should(ContainSubstring(fmt.Sprintf("%s [FAILED]", denoter)))
378378
Ω(output).ShouldNot(ContainSubstring("More_ginkgo_tests Suite"))
@@ -392,7 +392,7 @@ var _ = Describe("Running Specs", func() {
392392

393393
outputLines := strings.Split(output, "\n")
394394
Ω(outputLines[0]).Should(ContainSubstring("Skipping ./no_tagged_tests (no test files)"))
395-
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
395+
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
396396
Ω(outputLines[2]).Should(ContainSubstring("Failed to compile does_not_compile:"))
397397
Ω(output).ShouldNot(ContainSubstring("More_ginkgo_tests Suite"))
398398
Ω(output).Should(ContainSubstring("Test Suite Failed"))
@@ -412,7 +412,7 @@ var _ = Describe("Running Specs", func() {
412412

413413
outputLines := strings.Split(output, "\n")
414414
Ω(outputLines[0]).Should(ContainSubstring("Skipping ./no_tagged_tests (no test files)"))
415-
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
415+
Ω(outputLines[1]).Should(MatchRegexp(`\[\d+\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS! \d+(\.\d+)?[muµ]s PASS`, regexp.QuoteMeta(denoter)))
416416
Ω(outputLines[2]).Should(ContainSubstring("Failed to compile does_not_compile:"))
417417
Ω(output).Should(MatchRegexp(`\[\d+\] Failing_ginkgo_tests Suite - 2/2 specs`))
418418
Ω(output).Should(ContainSubstring(fmt.Sprintf("%s [FAILED]", denoter)))

‎integration/verbose_and_succinct_test.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ var _ = Describe("Verbose And Succinct Mode", func() {
4242
Eventually(session).Should(gexec.Exit(0))
4343
output := session.Out.Contents()
4444

45-
Ω(output).Should(MatchRegexp(`\] Passing_ginkgo_tests Suite - 4/4 specs [%s]{4} SUCCESS!`, regexp.QuoteMeta(denoter)))
45+
Ω(output).Should(MatchRegexp(`\] Passing_ginkgo_tests Suite - 5/5 specs [%s]{5} SUCCESS!`, regexp.QuoteMeta(denoter)))
4646
Ω(output).Should(MatchRegexp(`\] More_ginkgo_tests Suite - 2/2 specs [%s]{2} SUCCESS!`, regexp.QuoteMeta(denoter)))
4747
})
4848
})
@@ -78,6 +78,14 @@ var _ = Describe("Verbose And Succinct Mode", func() {
7878
Ω(output).Should(ContainSubstring("emitting one By"))
7979
Ω(output).Should(ContainSubstring("emitting another By"))
8080
})
81+
82+
It("doesn't trigger the race detector", func() {
83+
session := startGinkgo(fm.PathTo("passing_ginkgo_tests"), "--no-color", "-v", "-race")
84+
Eventually(session).Should(gexec.Exit(0))
85+
output := session.Out.Contents()
86+
87+
Ω(output).Should(ContainSubstring("avoiding the race detector"))
88+
})
8189
})
8290

8391
Context("with -vv", func() {

‎reporters/default_reporter.go

+28-19
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"io"
1313
"runtime"
1414
"strings"
15+
"sync"
1516
"time"
1617

1718
"github.com/onsi/ginkgo/v2/formatter"
@@ -23,7 +24,7 @@ type DefaultReporter struct {
2324
writer io.Writer
2425

2526
// managing the emission stream
26-
lastChar string
27+
lastCharWasNewline bool
2728
lastEmissionWasDelimiter bool
2829

2930
// rendering
@@ -32,6 +33,7 @@ type DefaultReporter struct {
3233
formatter formatter.Formatter
3334

3435
runningInParallel bool
36+
lock *sync.Mutex
3537
}
3638

3739
func NewDefaultReporterUnderTest(conf types.ReporterConfig, writer io.Writer) *DefaultReporter {
@@ -46,12 +48,13 @@ func NewDefaultReporter(conf types.ReporterConfig, writer io.Writer) *DefaultRep
4648
conf: conf,
4749
writer: writer,
4850

49-
lastChar: "\n",
51+
lastCharWasNewline: true,
5052
lastEmissionWasDelimiter: false,
5153

5254
specDenoter: "•",
5355
retryDenoter: "↺",
5456
formatter: formatter.NewWithNoColorBool(conf.NoColor),
57+
lock: &sync.Mutex{},
5558
}
5659
if runtime.GOOS == "windows" {
5760
reporter.specDenoter = "+"
@@ -619,31 +622,37 @@ func (r *DefaultReporter) emitSource(indent uint, fc types.FunctionCall) {
619622

620623
/* Emitting to the writer */
621624
func (r *DefaultReporter) emit(s string) {
622-
if len(s) > 0 {
623-
r.lastChar = s[len(s)-1:]
624-
r.lastEmissionWasDelimiter = false
625-
r.writer.Write([]byte(s))
626-
}
625+
r._emit(s, false, false)
627626
}
628627

629628
func (r *DefaultReporter) emitBlock(s string) {
630-
if len(s) > 0 {
631-
if r.lastChar != "\n" {
632-
r.emit("\n")
633-
}
634-
r.emit(s)
635-
if r.lastChar != "\n" {
636-
r.emit("\n")
637-
}
638-
}
629+
r._emit(s, true, false)
639630
}
640631

641632
func (r *DefaultReporter) emitDelimiter(indent uint) {
642-
if r.lastEmissionWasDelimiter {
633+
r._emit(r.fi(indent, "{{gray}}%s{{/}}", strings.Repeat("-", 30)), true, true)
634+
}
635+
636+
// a bit ugly - but we're trying to minimize locking on this hot codepath
637+
func (r *DefaultReporter) _emit(s string, block bool, isDelimiter bool) {
638+
if len(s) == 0 {
639+
return
640+
}
641+
r.lock.Lock()
642+
defer r.lock.Unlock()
643+
if isDelimiter && r.lastEmissionWasDelimiter {
643644
return
644645
}
645-
r.emitBlock(r.fi(indent, "{{gray}}%s{{/}}", strings.Repeat("-", 30)))
646-
r.lastEmissionWasDelimiter = true
646+
if block && !r.lastCharWasNewline {
647+
r.writer.Write([]byte("\n"))
648+
}
649+
r.lastCharWasNewline = (s[len(s)-1:] == "\n")
650+
r.writer.Write([]byte(s))
651+
if block && !r.lastCharWasNewline {
652+
r.writer.Write([]byte("\n"))
653+
r.lastCharWasNewline = true
654+
}
655+
r.lastEmissionWasDelimiter = isDelimiter
647656
}
648657

649658
/* Rendering text */

0 commit comments

Comments
 (0)
Please sign in to comment.