Skip to content

test: cover nil target entries in bundle debug list-targets#5206

Open
pavloKozlov wants to merge 1 commit intomainfrom
test/list-targets-nil-check
Open

test: cover nil target entries in bundle debug list-targets#5206
pavloKozlov wants to merge 1 commit intomainfrom
test/list-targets-nil-check

Conversation

@pavloKozlov
Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to fix: guard against nil target entries in bundle debug list-targets #5203. Adds a unit test for collectTargets that exercises the nil-entry path so the regression is locked in.
  • Test stays at the function level because no YAML pattern I tried produces a nil *config.Target entry through the loader pipeline (staging:, staging: null, staging: ~, staging: {} all yield non-nil entries). Direct unit coverage is the practical guard.

Test plan

  • go test ./cmd/bundle/debug -run TestCollectTargetsHandlesNilEntries passes.
  • Reverting the nil guard in collectTargets makes the test fail with a nil pointer dereference.

Copy link
Copy Markdown
Contributor

@janniklasrose janniklasrose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the follow-up! unlucky the initial acceptance test already had {} but that didn't cover it

@pavloKozlov pavloKozlov force-pushed the test/list-targets-nil-check branch from 58c6da6 to 08720c6 Compare May 7, 2026 13:27
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 7, 2026 13:28 — with GitHub Actions Inactive
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 7, 2026 13:28 — with GitHub Actions Inactive
@pavloKozlov pavloKozlov force-pushed the test/list-targets-nil-check branch from 08720c6 to e5105be Compare May 8, 2026 10:51
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 8, 2026 10:52 — with GitHub Actions Inactive
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 8, 2026 10:52 — with GitHub Actions Inactive
Replaces "staging: {}" with "staging:" in the existing acceptance test.
A null-bodied target follows the YAML→typed code path that produced the
nil *config.Target panic; with the guard in collectTargets the command
now lists the target by name instead of crashing.
@pavloKozlov pavloKozlov force-pushed the test/list-targets-nil-check branch from e5105be to e8e7b52 Compare May 8, 2026 12:14
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 8, 2026 12:15 — with GitHub Actions Inactive
@pavloKozlov pavloKozlov temporarily deployed to test-trigger-is May 8, 2026 12:15 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants