Add fail message for non-existent hostname#2785
Conversation
mislav
left a comment
There was a problem hiding this comment.
Thank you! The error message text needs changing, though.
Did you consider also making a test in status_test.go?
pkg/cmd/auth/status/status.go
Outdated
|
|
||
| if !isHostnameFound { | ||
| fmt.Fprintf(stderr, | ||
| "You are not logged into any GitHub hosts. Run %s to authenticate.\n", cs.Bold(fmt.Sprintf("gh auth login -h %s", opts.Hostname))) |
There was a problem hiding this comment.
We already print this message above:
cli/pkg/cmd/auth/status/status.go
Lines 71 to 74 in 4750e4a
The message you added here isn't accurate for the condition we're testing. The issue you've set to fix is to warn that a hostname passed via the -h argument wasn't found. How about something like this?
"Hostname %q not found among authenticated GitHub hosts"
There was a problem hiding this comment.
@mislav It seems that the code in status_test.go silently ignores the stderr message in favour of wantErr.
cli/pkg/cmd/auth/status/status_test.go
Lines 239 to 243 in 4750e4a
Since we return SilentError we have to make wantErr non-nil, and that forces condition on L241 to occur, thus pre-maturely returning, avoiding a check on wantErrOut. Any suggestions around this?
mislav
left a comment
There was a problem hiding this comment.
This looks good! Thank you.
|
@samcoe to fix conflicts and merge |
Fixes #2775.