Skip to content

Commit 7c36e19

Browse files
Move verbose out of testPlan; pass as parameter to script()
testPlan is a pure data value describing which suites to run. Verbosity is a rendering concern: it only affects the generated shell script, not what tests are selected. Changing the signature to script(verbose bool) makes this explicit at the call site and removes the hidden coupling between plan construction and script rendering. ai-assisted=yes Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b80e9da commit 7c36e19

2 files changed

Lines changed: 17 additions & 20 deletions

File tree

internal/test/container.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,8 @@ type Configuration struct {
6767
// setup that must succeed before any suite, and an ordered list of named
6868
// suites each of which is run as an independent unit.
6969
type testPlan struct {
70-
setup []string // fail-fast preamble (git config, etc.)
71-
suites []suiteStep
72-
verbose bool // when true: timestamps before each suite; npm output visible
70+
setup []string // fail-fast preamble (git config, etc.)
71+
suites []suiteStep
7372
}
7473

7574
// suiteStep is one test suite — migrations, stability, or manifest.
@@ -93,8 +92,7 @@ func (configuration Configuration) commands() (testPlan, error) {
9392
tileDirName := filepath.Base(configuration.AbsoluteTileDirectory)
9493

9594
plan := testPlan{
96-
setup: []string{"git config --global --add safe.directory '*'"},
97-
verbose: configuration.Verbose,
95+
setup: []string{"git config --global --add safe.directory '*'"},
9896
}
9997

10098
if configuration.RunMigrations || configuration.RunAll {
@@ -144,8 +142,9 @@ func (configuration Configuration) commands() (testPlan, error) {
144142
// Each suite runs in a subshell so cd calls don't leak between suites. Exit
145143
// codes are captured individually. When more than one suite is selected a
146144
// colored pass/fail summary is printed at the end. The script always exits
147-
// non-zero if any suite failed.
148-
func (p testPlan) script() string {
145+
// non-zero if any suite failed. When verbose is true, start/end timestamps
146+
// are echoed before and after each suite.
147+
func (p testPlan) script(verbose bool) string {
149148
var b strings.Builder
150149

151150
// Global setup — fail fast on any error.
@@ -160,12 +159,12 @@ func (p testPlan) script() string {
160159

161160
// One subshell per suite; exit code and end-time stored in _exitN / _timeN.
162161
for i, s := range p.suites {
163-
if p.verbose {
162+
if verbose {
164163
fmt.Fprintf(&b, "\necho \"[$(date '+%%H:%%M:%%S')] Starting: %s\"\n", s.name)
165164
}
166165
fmt.Fprintf(&b, "\n(%s); _exit%d=$?\n", strings.Join(s.cmds, " && "), i)
167166
fmt.Fprintf(&b, "_time%d=$(date '+%%H:%%M:%%S')\n", i)
168-
if p.verbose {
167+
if verbose {
169168
fmt.Fprintf(&b, "echo \"[$_time%d] Completed: %s\"\n", i, s.name)
170169
}
171170
}
@@ -233,7 +232,7 @@ func runTest(ctx context.Context, w io.Writer, dockerDaemon mobyClient, configur
233232
tileDir := filepath.Base(configuration.AbsoluteTileDirectory)
234233
envVars := getTileTestEnvVars(configuration.AbsoluteTileDirectory, tileDir, envMap)
235234

236-
return startAndWaitContainer(ctx, w, dockerDaemon, plan.script(), envVars, parentDir, configuration.Verbose)
235+
return startAndWaitContainer(ctx, w, dockerDaemon, plan.script(configuration.Verbose), envVars, parentDir, configuration.Verbose)
237236
}
238237

239238
// buildTestImage builds the kiln test Docker image, forwarding Artifactory

internal/test/container_test.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func TestTestPlan_script_includesSummaryForMultipleSuites(t *testing.T) {
160160
},
161161
}
162162

163-
script := plan.script()
163+
script := plan.script(false)
164164

165165
// Each suite runs in a subshell with captured exit code.
166166
require.Contains(t, script, "); _exit0=$?")
@@ -199,7 +199,7 @@ func TestTestPlan_script_omitsSummaryForSingleSuite(t *testing.T) {
199199
},
200200
}
201201

202-
script := plan.script()
202+
script := plan.script(false)
203203

204204
// No summary text for single suite.
205205
require.NotContains(t, script, "Passed")
@@ -213,7 +213,7 @@ func TestTestPlan_script_emptyWithNoSuites(t *testing.T) {
213213
plan := testPlan{
214214
setup: []string{"git config --global --add safe.directory '*'"},
215215
}
216-
script := plan.script()
216+
script := plan.script(false)
217217
require.Contains(t, script, "git config")
218218
require.NotContains(t, script, "_exit0")
219219
require.NotContains(t, script, "_overall")
@@ -294,15 +294,14 @@ func TestConfiguration_commands_usesNpmInstallWithoutLockfile(t *testing.T) {
294294

295295
func TestTestPlan_script_verbose_addsStartAndEndTimestamps(t *testing.T) {
296296
plan := testPlan{
297-
setup: []string{"setup cmd"},
298-
verbose: true,
297+
setup: []string{"setup cmd"},
299298
suites: []suiteStep{
300299
{name: "Migration Tests", cmds: []string{"npm test"}},
301300
{name: "Stability Tests", cmds: []string{"ginkgo stability"}},
302301
},
303302
}
304303

305-
script := plan.script()
304+
script := plan.script(true)
306305

307306
// Start and end echo lines present for each suite.
308307
require.Contains(t, script, "Starting: Migration Tests")
@@ -313,12 +312,11 @@ func TestTestPlan_script_verbose_addsStartAndEndTimestamps(t *testing.T) {
313312

314313
func TestTestPlan_script_noStartEndEchoWhenNotVerbose(t *testing.T) {
315314
plan := testPlan{
316-
setup: []string{"setup cmd"},
317-
verbose: false,
318-
suites: []suiteStep{{name: "Migration Tests", cmds: []string{"npm test"}}},
315+
setup: []string{"setup cmd"},
316+
suites: []suiteStep{{name: "Migration Tests", cmds: []string{"npm test"}}},
319317
}
320318

321-
script := plan.script()
319+
script := plan.script(false)
322320

323321
// No verbose echo lines; end-time variable is still captured for potential summary use.
324322
require.NotContains(t, script, "Starting:")

0 commit comments

Comments
 (0)