search

AlecAivazis / survey

A golang library for building interactive and accessible prompts with full support for windows and posix terminals.
MIT License
4.07k stars 350 forks source link

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

Open SirRegion opened 2 years ago

SirRegion commented 2 years ago

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.

mislav commented 2 years ago

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?

SirRegion commented 2 years ago

Sure!

mislav commented 2 years ago

@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.

SirRegion commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.