Skip to content

ci: acceptance-telemetry never runs the two login telemetry tests (missing HOOKDECK_CLI_TESTING_CLI_KEY) #422

Description

@leggetter

What

The acceptance-telemetry job in .github/workflows/acceptance.yml does not set HOOKDECK_CLI_TESTING_CLI_KEY, so the two tests in test/acceptance/telemetry_test.go that need it skip themselves on every run. The job still reports green.

  acceptance-telemetry:
    runs-on: ubuntu-latest
    env:
      ACCEPTANCE_SLICE: "0"
      HOOKDECK_CLI_TESTING_API_KEY: ${{ secrets.HOOKDECK_CLI_TESTING_API_KEY }}
      HOOKDECK_CLI_TELEMETRY_DISABLED: "0"

The matrix acceptance job does set it (acceptance.yml:37), but it does not build -tags=telemetry, so it never compiles these tests.

Affected:

  • TestTelemetryLoginProxy — telemetry_test.go:63-65
  • TestTelemetryLoginCommandFlagsProxy — telemetry_test.go:84-86

Evidence

From a green acceptance-telemetry run on PR #421 (job 106696045003), 62 pass / 8 skip:

$ gh run view --job=106696045003 --log | grep -E -- '--- SKIP:'
--- SKIP: TestTelemetryLoginProxy (0.00s)
--- SKIP: TestTelemetryLoginCommandFlagsProxy (0.00s)
...

This has been the case since the tests were added. Parsing every revision of acceptance.yml and test-acceptance.yml with a YAML parser: no job that has ever run -tags=telemetry has had HOOKDECK_CLI_TESTING_CLI_KEY in its env:, and no matrix slice has ever carried the telemetry tag. The skip guard has been in telemetry_test.go since the commit that introduced these tests (#271).

Why it matters

command_flags is the practical case. TestTelemetryLoginCommandFlagsProxy is the only assertion on that JSON tag anywhere in the tree:

$ grep -rn command_flags --include='*.go' .
test/acceptance/telemetry_test.go:78    (comment)
test/acceptance/telemetry_test.go:115
test/acceptance/telemetry_test.go:119
pkg/hookdeck/telemetry.go:38

The other CLITelemetry JSON tags are pinned by literal key name in pkg/hookdeck/telemetry_test.go, which runs under the required unit-test check. command_flags is not, so renaming it passes every check in CI while any analytics keyed on the old name go quiet.

Suggested fix

One line in that job's env:, mirroring acceptance.yml:37:

      HOOKDECK_CLI_TESTING_CLI_KEY: ${{ secrets.HOOKDECK_CLI_TESTING_CLI_KEY }}

The comment at acceptance.yml:29-36 applies here too: that key has to belong to the test-only account, because acceptance logs on this repository are public.

Worth confirming the two tests actually pass once the key is wired in — they have not run before, so this may surface real failures rather than just turning two skips into two passes.

Out of scope elsewhere

Found while reviewing #421 (a docs-only change adding a review skill). Noted there rather than fixed, since it is a CI change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions