Repository navigation
feat(cli): implement exp task status command - #19533
Conversation
- Implements `coder exp task status` - Adds `testutil.MustURL` helper
| args []string | ||
| expectOutput string | ||
| expectError string | ||
| hf func(context.Context) func(http.ResponseWriter, *http.Request) |
There was a problem hiding this comment.
review: I elected to mock the API response instead of standing up a whole coderdtest. We have too many of these as it is.
| "testing" | ||
| ) | ||
|
|
||
| func MustURL(t testing.TB, raw string) *url.URL { |
There was a problem hiding this comment.
review: I figured this might be useful.
| State: codersdk.TaskStateWorking, | ||
| }, | ||
| }) | ||
| c.Add(1) |
There was a problem hiding this comment.
I'm just sad I couldn't write c++ here 😢
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
I think it'd be nice to have a /status endpoint in coderd that allows follow without polling, but I'm fine with the current implementation for now. Some comments/suggestions inline.
|
|
||
| // TODO: right now tasks are still "workspaces" under the hood. | ||
| // We should update this once we have a proper task model. | ||
| ws, err := namedWorkspace(ctx, client, id) |
There was a problem hiding this comment.
Is this lookup necessary, is it mainly to support e.g. workspace name rather than plain uuid lookups?
We should perhaps consider supporting name lookup in the get task endpoint.
There was a problem hiding this comment.
Yeah this is basically for convenience due to the current task <-> workspace relationship
|
|
||
| var ( | ||
| ctx = testutil.Context(t, testutil.WaitShort) | ||
| now = time.Now().UTC() // TODO: replace with quartz |
There was a problem hiding this comment.
review: this is unfortunately flake-prone unless we just test the JSON output which omits the relative time.
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
I think switching to the table approach was a good call, nice. A few suggestions pertaining to JSON output but otherwise LGTM.
| Handler: func(i *serpent.Invocation) error { | ||
| ctx := i.Context() | ||
| ec := codersdk.NewExperimentalClient(client) | ||
| id := i.Args[0] |
There was a problem hiding this comment.
Just realized this could support multiple args (tasks) as well, and watching each. But let's leave that for future.
johnstcn
added
the
cherry-pick/v2.26
Closes coder/internal#900
coder exp task statustestutil.MustURLhelperNo robots were harmed in the making of this PR.