Skip to content

Commit 976a5c0

Browse files
committed
strip out symbols when running ginkgo
...unless we need them, in which case don't. this, at last, resolves a years-long performance gap between ginkgo and go test 🎉
1 parent 99e2fe2 commit 976a5c0

File tree

11 files changed

+78
-12
lines changed

11 files changed

+78
-12
lines changed

ginkgo/build/build_command.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func buildSpecs(args []string, cliConfig types.CLIConfig, goFlagsConfig types.Go
4444
internal.VerifyCLIAndFrameworkVersion(suites)
4545

4646
opc := internal.NewOrderedParallelCompiler(cliConfig.ComputedNumCompilers())
47-
opc.StartCompiling(suites, goFlagsConfig)
47+
opc.StartCompiling(suites, goFlagsConfig, true)
4848

4949
for {
5050
suiteIdx, suite := opc.Next()

ginkgo/command/command.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func (c Command) Run(args []string, additionalArgs []string) {
2626
}
2727
for _, arg := range args {
2828
if strings.HasPrefix(arg, "-") {
29-
AbortWith("Malformed arguments - make sure all flags appear {{bold}}after{{/}} the Ginkgo subcommand and {{bold}}before{{/}} your list of packages.\n{{gray}}e.g. 'ginkgo run -p my_package' is valid `ginkgo -p run my_package` is not.{{/}}")
29+
AbortWith(types.GinkgoErrors.FlagAfterPositionalParameter().Error())
3030
}
3131
}
3232
c.Command(args, additionalArgs)

ginkgo/internal/compile.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/onsi/ginkgo/v2/types"
1212
)
1313

14-
func CompileSuite(suite TestSuite, goFlagsConfig types.GoFlagsConfig) TestSuite {
14+
func CompileSuite(suite TestSuite, goFlagsConfig types.GoFlagsConfig, preserveSymbols bool) TestSuite {
1515
if suite.PathToCompiledTest != "" {
1616
return suite
1717
}
@@ -46,7 +46,7 @@ func CompileSuite(suite TestSuite, goFlagsConfig types.GoFlagsConfig) TestSuite
4646
suite.CompilationError = fmt.Errorf("Failed to get relative path from package to the current working directory:\n%s", err.Error())
4747
return suite
4848
}
49-
args, err := types.GenerateGoTestCompileArgs(goFlagsConfig, "./", pathToInvocationPath)
49+
args, err := types.GenerateGoTestCompileArgs(goFlagsConfig, "./", pathToInvocationPath, preserveSymbols)
5050
if err != nil {
5151
suite.State = TestSuiteStateFailedToCompile
5252
suite.CompilationError = fmt.Errorf("Failed to generate go test compile flags:\n%s", err.Error())
@@ -120,7 +120,7 @@ func NewOrderedParallelCompiler(numCompilers int) *OrderedParallelCompiler {
120120
}
121121
}
122122

123-
func (opc *OrderedParallelCompiler) StartCompiling(suites TestSuites, goFlagsConfig types.GoFlagsConfig) {
123+
func (opc *OrderedParallelCompiler) StartCompiling(suites TestSuites, goFlagsConfig types.GoFlagsConfig, preserveSymbols bool) {
124124
opc.stopped = false
125125
opc.idx = 0
126126
opc.numSuites = len(suites)
@@ -135,7 +135,7 @@ func (opc *OrderedParallelCompiler) StartCompiling(suites TestSuites, goFlagsCon
135135
stopped := opc.stopped
136136
opc.mutex.Unlock()
137137
if !stopped {
138-
suite = CompileSuite(suite, goFlagsConfig)
138+
suite = CompileSuite(suite, goFlagsConfig, preserveSymbols)
139139
}
140140
c <- suite
141141
}

ginkgo/performance/performance_suite_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ var _ = SynchronizedAfterSuite(func() {}, func() {
5151
})
5252

5353
/*
54-
GoModCacheManager sets up a new GOMODCACHE and knows how to clear it
55-
This allows us to bust the go mod cache.
54+
GoModCacheManager sets up a new GOMODCACHE and knows how to clear it
55+
This allows us to bust the go mod cache.
5656
*/
5757
type GoModCacheManager struct {
5858
Path string
@@ -302,7 +302,7 @@ func RunScenarioWithGinkgoInternals(stopwatch *gmeasure.Stopwatch, settings Scen
302302
for suite := range compile {
303303
if !suite.State.Is(internal.TestSuiteStateCompiled) {
304304
subStopwatch := stopwatch.NewStopwatch()
305-
suite = internal.CompileSuite(suite, goFlagsConfig)
305+
suite = internal.CompileSuite(suite, goFlagsConfig, false)
306306
subStopwatch.Record("compile-test: "+suite.PackageName, annotation)
307307
Ω(suite.CompilationError).Should(BeNil())
308308
}

ginkgo/run/run_command.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ OUTER_LOOP:
107107
}
108108

109109
opc := internal.NewOrderedParallelCompiler(r.cliConfig.ComputedNumCompilers())
110-
opc.StartCompiling(suites, r.goFlagsConfig)
110+
opc.StartCompiling(suites, r.goFlagsConfig, false)
111111

112112
SUITE_LOOP:
113113
for {

ginkgo/watch/watch_command.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func (w *SpecWatcher) WatchSpecs(args []string, additionalArgs []string) {
153153
}
154154

155155
func (w *SpecWatcher) compileAndRun(suite internal.TestSuite, additionalArgs []string) internal.TestSuite {
156-
suite = internal.CompileSuite(suite, w.goFlagsConfig)
156+
suite = internal.CompileSuite(suite, w.goFlagsConfig, false)
157157
if suite.State.Is(internal.TestSuiteStateFailedToCompile) {
158158
fmt.Println(suite.CompilationError.Error())
159159
return suite
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package symbol_fixture_test
2+
3+
import (
4+
"fmt"
5+
"os/exec"
6+
"testing"
7+
8+
. "github.com/onsi/ginkgo/v2"
9+
. "github.com/onsi/gomega"
10+
)
11+
12+
func TestSymbolFixture(t *testing.T) {
13+
RegisterFailHandler(Fail)
14+
RunSpecs(t, "SymbolFixture Suite")
15+
}
16+
17+
var _ = It("prints out its symbols", func() {
18+
cmd := exec.Command("go", "tool", "nm", "symbol_fixture.test")
19+
output, err := cmd.CombinedOutput()
20+
Expect(err).NotTo(HaveOccurred())
21+
fmt.Println(string(output))
22+
})

integration/precompiled_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ var _ = Describe("ginkgo build", func() {
2525
Ω(fm.PathTo("passing_ginkgo_tests", "passing_ginkgo_tests.test")).Should(BeAnExistingFile())
2626
})
2727

28+
It("should have the symbols in the compiled binary", func() {
29+
cmd := exec.Command("go", "tool", "nm", fm.PathTo("passing_ginkgo_tests", "passing_ginkgo_tests.test"))
30+
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
31+
Ω(err).ShouldNot(HaveOccurred())
32+
Eventually(session).Should(gexec.Exit(0))
33+
Ω(session).Should(gbytes.Say("github.com/onsi/ginkgo/v2.It")) // a symbol from ginkgo
34+
})
35+
2836
It("should be possible to run the test binary directly", func() {
2937
cmd := exec.Command("./passing_ginkgo_tests.test")
3038
cmd.Dir = fm.PathTo("passing_ginkgo_tests")

integration/run_test.go

+21
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,27 @@ var _ = Describe("Running Specs", func() {
458458
})
459459
})
460460

461+
Describe("optimizing build times by stripping symbols", func() {
462+
BeforeEach(func() {
463+
fm.MountFixture("symbol")
464+
})
465+
Context("when no symbols are required (i.e. no profiling)", func() {
466+
It("should not have symbols", func() {
467+
session := startGinkgo(fm.PathTo("symbol"), "--no-color")
468+
Eventually(session).Should(gexec.Exit(0))
469+
Ω(session).ShouldNot(gbytes.Say("github.com/onsi/ginkgo/v2.It")) // a symbol from ginkgo
470+
})
471+
})
472+
473+
Context("when symbols are required (i.e. with profiling)", func() {
474+
It("should have symbols", func() {
475+
session := startGinkgo(fm.PathTo("symbol"), "--no-color", "--cpuprofile=cpu.out")
476+
Eventually(session).Should(gexec.Exit(0))
477+
Ω(session).Should(gbytes.Say("github.com/onsi/ginkgo/v2.It")) // a symbol from ginkgo
478+
})
479+
})
480+
})
481+
461482
Context("when there is a version mismatch between the cli and the test package", func() {
462483
It("emits a useful error and tries running", func() {
463484
fm.MountFixture(("version_mismatch"))

types/config.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,10 @@ func (g GoFlagsConfig) BinaryMustBePreserved() bool {
231231
return g.BlockProfile != "" || g.CPUProfile != "" || g.MemProfile != "" || g.MutexProfile != ""
232232
}
233233

234+
func (g GoFlagsConfig) NeedsSymbols() bool {
235+
return g.BinaryMustBePreserved()
236+
}
237+
234238
// Configuration that were deprecated in 2.0
235239
type deprecatedConfig struct {
236240
DebugParallel bool
@@ -640,7 +644,7 @@ func VetAndInitializeCLIAndGoConfig(cliConfig CLIConfig, goFlagsConfig GoFlagsCo
640644
}
641645

642646
// GenerateGoTestCompileArgs is used by the Ginkgo CLI to generate command line arguments to pass to the go test -c command when compiling the test
643-
func GenerateGoTestCompileArgs(goFlagsConfig GoFlagsConfig, packageToBuild string, pathToInvocationPath string) ([]string, error) {
647+
func GenerateGoTestCompileArgs(goFlagsConfig GoFlagsConfig, packageToBuild string, pathToInvocationPath string, preserveSymbols bool) ([]string, error) {
644648
// if the user has set the CoverProfile run-time flag make sure to set the build-time cover flag to make sure
645649
// the built test binary can generate a coverprofile
646650
if goFlagsConfig.CoverProfile != "" {
@@ -663,6 +667,10 @@ func GenerateGoTestCompileArgs(goFlagsConfig GoFlagsConfig, packageToBuild strin
663667
goFlagsConfig.CoverPkg = strings.Join(adjustedCoverPkgs, ",")
664668
}
665669

670+
if !goFlagsConfig.NeedsSymbols() && goFlagsConfig.LDFlags == "" && !preserveSymbols {
671+
goFlagsConfig.LDFlags = "-w -s"
672+
}
673+
666674
args := []string{"test", "-c", packageToBuild}
667675
goArgs, err := GenerateFlagArgs(
668676
GoBuildFlags,

types/errors.go

+7
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,13 @@ func (g ginkgoErrors) ExpectFilenameNotPath(flag string, path string) error {
636636
}
637637
}
638638

639+
func (g ginkgoErrors) FlagAfterPositionalParameter() error {
640+
return GinkgoError{
641+
Heading: "Malformed arguments - detected a flag after the package liste",
642+
Message: "Make sure all flags appear {{bold}}after{{/}} the Ginkgo subcommand and {{bold}}before{{/}} your list of packages (or './...').\n{{gray}}e.g. 'ginkgo run -p my_package' is valid but `ginkgo -p run my_package` is not.\n{{gray}}e.g. 'ginkgo -p -vet ./...' is valid but 'ginkgo -p ./... -vet' is not{{/}}",
643+
}
644+
}
645+
639646
/* Stack-Trace parsing errors */
640647

641648
func (g ginkgoErrors) FailedToParseStackTrace(message string) error {

0 commit comments

Comments
 (0)