Fix job unblock data precedence - #927
Conversation
| @@ -89,14 +89,16 @@ func (c *UnblockCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { | |||
| } | |||
|
|
|||
| func (c *UnblockCmd) unblockFields() (map[string]any, error) { | |||
There was a problem hiding this comment.
Nice work!
One small suggestion on the test though, right now it works by temporarily swapping out the program's input with a placeholder one, then switching it back after the test is done. That works, but I think it would be safer to just enter the test input directly into the function instead of swapping anything out. The reason is that if another test happens to run around the same time, they could end up interfering with each other.
| } | ||
| } | ||
|
|
||
| func TestUnblockFieldsPrefersDataOverEmptyNonTTYStdin(t *testing.T) { |
There was a problem hiding this comment.
The other thing is, right now there's only one test, and it covers the case where both --data and manually entered data are given, confirming that --data correctly wins. But this change touches two other situations as well, when data is entered manually with no --data at all, and when neither one is provided, so it'd be worth adding tests for those too. That way we can be confident the reordering didn't affect anything that already worked before this fix.
None of this is blocking, just some points to improve on before it merges
e075178 to
8c68a8d
Compare
|
Cleaned up the PR, sorry for the delay. |
2 participants
Summary
--datavalue over stdin inbk job unblock--datais absent--dataprecedence, stdin fallback, and no-input behaviorFixes #926.
Testing
go test -race ./cmd/jobgo vet ./cmd/jobgit diff --checkI also ran
go test ./...; all relevant packages passed, but the existing keyring tests failed because this development environment has an active shared-memory credential store.