Conversation
mislav
left a comment
There was a problem hiding this comment.
Nice work!
I'm considering accepting a prompt in the
Register*functions as the first argument so the caller doesn't have to do the NoSuchPromptErr themselves.
I'd like to see that 👍
samcoe
left a comment
There was a problem hiding this comment.
I like this approach. Agreed that adding the prompt as the first argument to Register functions would be a nice improvement.
034b5d0 to
21fbe57
Compare
0173f97 to
10dd74b
Compare
| type Prompter interface { | ||
| Select(string, string, []string) (int, error) | ||
| MultiSelect(string, string, []string) (int, error) | ||
| MultiSelect(string, string, []string) ([]string, error) |
There was a problem hiding this comment.
For symmetry with Select, would we return []int here?
There was a problem hiding this comment.
i'm torn about it. no code is actually relying on the new MultiSelect so i haven't played with it (hence the bug) to see how cumbersome a list of ints is to work with. i'll make a call when i'm actually porting a MultiSelect which will be soon.
| ConfirmDeletionStubs []ConfirmDeletionStub | ||
| } | ||
|
|
||
| // TODO thread safety |
There was a problem hiding this comment.
maybe it's okay if these methods aren't threadsafe, prompts in the UI cannot happen in parallel anyway?
This PR implements a more robust way to make assertions about prompts; awkwardy called
MockPrompter. It wraps the auto-generatedPrompterMockand adds functions for registering expectations and verifying them.to use it in a test:
The
MockPrompterensures that:and avoids any
interface{}shenanigans.This PR includes updates to two test files to illustrate the new approach.