Skip to content

chore(cli): add single CRUD-style test for tasks#20385

Merged
johnstcn merged 1 commit into
mainfrom
cj/cli-exp-tasks-coderdtest
Oct 22, 2025
Merged

chore(cli): add single CRUD-style test for tasks#20385
johnstcn merged 1 commit into
mainfrom
cj/cli-exp-tasks-coderdtest

Conversation

@johnstcn
Copy link
Copy Markdown
Member

@johnstcn johnstcn commented Oct 20, 2025

Replaces duplicated tasks tests with Adds a single CRUD-style test for tasks CLI using a single coderdtest instance.

@johnstcn johnstcn self-assigned this Oct 20, 2025
Copy link
Copy Markdown
Contributor

@DanielleMaywood DanielleMaywood left a comment

Choose a reason for hiding this comment

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

Approving although I think we lose test coverage here

@johnstcn
Copy link
Copy Markdown
Member Author

Approving although I think we lose test coverage here

I can keep the failure cases around if that helps?

Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Some suggestions and nits but integration test looks good otherwise.

I'm not so sure about deleting the existing tests for send/logs though as there are a lot of cases not tested in the integration.

Comment thread cli/exp_task_test.go Outdated
Comment thread cli/exp_task_test.go Outdated
Comment thread cli/exp_task_test.go Outdated
Comment thread cli/exp_task_test.go Outdated
Comment thread cli/exp_task_test.go Outdated
Comment thread cli/exp_task_test.go Outdated
@johnstcn
Copy link
Copy Markdown
Member Author

johnstcn commented Oct 21, 2025

I'm not so sure about deleting the existing tests for send/logs though as there are a lot of cases not tested in the integration.

I reverted the commits that delete them. We may wish to consolidate the coderdtest invocations but that can be a later PR.

@johnstcn johnstcn requested a review from mafredri October 21, 2025 09:07
@johnstcn johnstcn force-pushed the cj/cli-exp-tasks-coderdtest branch 2 times, most recently from 6369650 to ec9e273 Compare October 22, 2025 11:36
@johnstcn johnstcn force-pushed the cj/cli-exp-tasks-coderdtest branch from ec9e273 to 562bbb4 Compare October 22, 2025 11:40
Comment thread cli/exp_task_test.go
require.NoError(t, json.NewDecoder(strings.NewReader(stdout)).Decode(&task), "should unmarshal task status")
require.Equal(t, task.Name, taskName, "task name should match")
// NOTE: task status changes type, this is so this test works with both old and new model
require.Contains(t, []string{"active", "running"}, string(task.Status), "task should be active")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should work both pre- and post-refactor.

@johnstcn johnstcn merged commit 5ecab7b into main Oct 22, 2025
25 checks passed
@johnstcn johnstcn deleted the cj/cli-exp-tasks-coderdtest branch October 22, 2025 15:02
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants