Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new uaa targets command to list all configured UAA targets (marking the active one), along with documentation and integration-style tests.
Changes:
- Introduces
targetsCobra command that prints all saved targets and marks the active target with*. - Adds Ginkgo/Gomega coverage for no-target, single-target, and multi-target scenarios.
- Adds user-facing documentation for the new command output and usage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/commands/targets.md | New documentation page for uaa targets, including expected output format. |
| cmd/targets.go | Implements the targets command and output formatting. |
| cmd/targets_test.go | Adds test coverage for targets command behavior across config states. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| output := string(session.Out.Contents()) | ||
| Expect(output).To(ContainSubstring("* ")) | ||
| Expect(output).To(ContainSubstring(secondURL)) | ||
| }) |
There was a problem hiding this comment.
This only checks that the output contains "* ", but it doesn’t verify that only one target is marked active (the test would still pass if multiple lines had *). Consider asserting the count of active markers is exactly 1, and that it appears on the active target’s line.
| if k == cfg.ActiveTargetName { | ||
| marker = "* " | ||
| } | ||
| log.Info(fmt.Sprintf("%s%d: %s", marker, i+1, target.BaseUrl)) |
There was a problem hiding this comment.
Minor: cli.Logger already provides Infof, so you can avoid the extra fmt.Sprintf allocation/import here by using log.Infof(...) directly.
| # targets | ||
|
|
||
| [← Command Reference](../commands.md) | ||
|
|
||
| List all registered targets. The active target is marked with `*`. |
There was a problem hiding this comment.
targets.md is added, but the command isn’t discoverable from the command index (docs/commands.md) yet. Add a targets row under "Getting Started" (near target) so the new documentation page is reachable from the reference list.
| It("exits 0 and lists all targets with active marker on the second", func() { | ||
| session := runCommand("targets") | ||
|
|
||
| Eventually(session).Should(Exit(0)) | ||
| Expect(session.Out).To(Say(server.URL())) | ||
| Expect(session.Out).To(Say(secondURL)) | ||
| }) |
There was a problem hiding this comment.
This test description says the active marker is on the second target, but the assertions only check that both URLs appear. Please also assert that the * marker (and index) are printed on the active target’s line (e.g., matching the full line containing * + index + second URL).
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
No description provided.