survey icon indicating copy to clipboard operation
survey copied to clipboard

Added an interface for all the public functions of the survey package

Open SirRegion opened this issue 2 years ago • 6 comments

This PR adds an interface for survey that can be used instead of calling the functions directly.

It also includes a struct with no contents hat implements the interface.

This lays the groundwork for adding a survey mock as discussed in issue #428.

SirRegion avatar Jun 02 '22 15:06 SirRegion

Thanks for taking this on! Since this is a work in progress, would you mind marking the PR as draft until it's ready for review?

mislav avatar Jun 08 '22 12:06 mislav

Sure!

SirRegion avatar Jun 10 '22 10:06 SirRegion

@SirRegion I meant, instead of changing the title of the PR to include the word "draft", actually making the PR as draft in GitHub web UI. I believe that the option to do it is in the right sidebar for the PR.

mislav avatar Jun 14 '22 16:06 mislav

Oh, thank you. I'm way more familiar with GitLab than Github, where adding "draft:" to an PR automatically converts it. I fixed it now :)

SirRegion avatar Jun 20 '22 07:06 SirRegion

I added an implementation for the mock (including tests) and some documentation in the readme on how to use it.

You can now start reviewing and merging if you think it's ready :)

SirRegion avatar Jun 20 '22 11:06 SirRegion

This looks promising! Thank you for working on it.

Over in the GitHub CLI codebase we have experience with mocking Survey using our own approach. One thing we learned is that our tests are more resilient over a longer period of time if we mock Survey in a way that the mocks are still valid even if the underlying implementation switches from Ask to AskOne (and vice versa). Also, we have ability to mock multiple sequential calls of Ask/AskOne. We do this by matching result stubs not to Ask/AskOne calls, but to prompt messages. What do you think of this approach, and would you be open if I helped port it over to what you are proposing in your PR?

Thanks for your suggestions. I basically just build an MVP that is enough for my purposes, but I would be happy to see you implement a more sturdy variant on top of the work I already did. I think it would be great to have both the ability to define the response for Ask/AskOne directly and also have the more complex way of returning an answer based on the prompt given.

SirRegion avatar Jun 21 '22 10:06 SirRegion