Skip to content

Commit 25095e3

Browse files
committed
test(cli): stop the telemetry report tests passing vacuously
Version is "dev" in a test binary, and a dev build has telemetry switched off entirely, so reportCommand returned at its first line in both tests that exercise it. They asserted that no child process was started, which was true for a reason that had nothing to do with the code under test: they would have passed just as well with the opt-out gate and the unresolved-command filter both deleted. The tests now make the binary look like a released one for their duration. A positive case comes with them, asserting that a resolved command with telemetry on really does start a child, so that a reportCommand which returned early for every input can no longer satisfy the pair. Verified by mutation: removing either the opt-out gate or the empty-command filter now fails the corresponding test, where before it did not. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 2dd3f4ba-458a-4d2b-8f0b-eb8f7804f4d0
1 parent 448b608 commit 25095e3

1 file changed

Lines changed: 27 additions & 0 deletions

File tree

‎app/cli/cmd/telemetry_spawn_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,18 @@ func assertNoChildOutput(t *testing.T, path, msg string) {
7373
assert.True(t, os.IsNotExist(err), msg)
7474
}
7575

76+
// withReleaseVersion makes the binary look like a released one for the duration of a test.
77+
// Version is "dev" by default, and a dev build has telemetry switched off entirely, so
78+
// without this every test of reportCommand would return at the first line and pass no
79+
// matter what the code under it did.
80+
func withReleaseVersion(t *testing.T) {
81+
t.Helper()
82+
83+
original := Version
84+
Version = "v1.2.3"
85+
t.Cleanup(func() { Version = original })
86+
}
87+
7688
// TestSpawnTelemetryFlushDeliversThePayload covers the process boundary itself: what the
7789
// parent writes is what the child reads on its stdin.
7890
func TestSpawnTelemetryFlushDeliversThePayload(t *testing.T) {
@@ -173,6 +185,7 @@ func TestReportCommandIsSilentWhenTelemetryIsDisabled(t *testing.T) {
173185

174186
for _, tc := range testCases {
175187
t.Run(tc.name, func(t *testing.T) {
188+
withReleaseVersion(t)
176189
t.Setenv(doNotTrackEnv, tc.value)
177190
out := stubFlushCommand(t, `cat > "$1"`)
178191

@@ -187,10 +200,24 @@ func TestReportCommandIsSilentWhenTelemetryIsDisabled(t *testing.T) {
187200
// subcommands, where cobra hands back the root command and there is no command path to
188201
// report.
189202
func TestReportCommandSkipsUnresolvedCommands(t *testing.T) {
203+
withReleaseVersion(t)
190204
t.Setenv(doNotTrackEnv, "")
191205
out := stubFlushCommand(t, `cat > "$1"`)
192206

193207
reportCommand(newTestRootCommand(), time.Second, nil)
194208

195209
assertNoChildOutput(t, out, "the root command has no command path and must not be reported")
196210
}
211+
212+
// TestReportCommandSpawnsForARealCommand is the positive case the two tests above are only
213+
// meaningful against: with telemetry on and a resolved command, a child really is started.
214+
// Without it, a reportCommand that returned early for every input would satisfy them both.
215+
func TestReportCommandSpawnsForARealCommand(t *testing.T) {
216+
withReleaseVersion(t)
217+
t.Setenv(doNotTrackEnv, "")
218+
out := stubFlushCommand(t, `cat > "$1"`)
219+
220+
reportCommand(newReportableCommand(), 1500*time.Millisecond, nil)
221+
222+
assert.Contains(t, string(waitForFile(t, out)), `"duration_ms":1500`)
223+
}

0 commit comments

Comments
 (0)