-
Notifications
You must be signed in to change notification settings - Fork 874
refactor: claim prebuilt workspace tests #17567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: claim prebuilt workspace tests #17567
Conversation
@@ -28,6 +29,13 @@ import ( | |||
"github.com/coder/coder/v2/testutil" | |||
) | |||
|
|||
type storeType int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of going this far, can we not just fold the error store into the storySpy
?
It could just take a claimErr
in its constructor, and optionally return it in ClaimPrebuiltWorkspace
.
That would simplify this implementation quite a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Follow-up to: #17458
Specifically it addresses these discussions: