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

Feature Request: Survey Mock #428

Open SirRegion opened 2 years ago

SirRegion commented 2 years ago

Hi @AlecAivazis, thank you for creating this great library.

I used it a lot in my work project, but I have now reached a point, where I spend a lot more time debugging than implementing features. The main reason: missing tests 😢.

So I started adding tests and implemented wrappers and fakes for all my os calls (or used a library that included those).

Survey is now the only dependency with side effects that I use that has no wrappers / mocks available. I tried writing them myself, but I got really lost when trying to correctly implement a mock that still correctly sets the responses I have given it. (Reflection is really not my strong suit 😆)

So my proposal would be the following:

1. Add a survey interface and struct

Adding a interface that defines all the functions survey provides and a struct that implements all those functions would enable the user to pass around the survey struct and use dependency injection instead of hardcoding access to the survey package.

It would also allow users to extend survey without having to wait until their pull request is merged into this repo. This would also allow storing configurations inside the struct, removing the requirement to pass them to each Ask or AskOne function.

2. Add a survey mock struct

Adding a survey mock struct that implements the interface, but does not actually show any output on the command line would enable users to create unit tests without having to rely on a virtual terminal. Not only would this speed up tests, but also improve their clarity.

For Example: You want to test your newly written function askRandomQuestion asks the predefined question "Are you there?" and if the user responds with "Yes". If you're using go-expect, you first have to set up the environment, pass stdio and stderr to survey and than carefully check the complete text on the terminal, to make sure everything is working correctly.

But if you would have access to a mock, you could simply call surveyMock.getLatestQuestion to confirm if the latest question really was "Are you there". And if you also want to check the result of the function, you cound simply set the response by calling surveyMock.setResponse("Yes") before executing the function and then confirm afterwards.

I know this would probably be a lot of work, but I would really love to have this feature in survey!

PS: I would be open to try to implement the first part, but that change does not make sense if the second part is not being considered. (And the second part would have to be someone else's responsibility, I don't know how to do it 😁)

AlecAivazis commented 2 years ago

Thank you for the kind words @SirRegion. I'm happy to know that you are enjoying survey. I have to give a lot of the credit to @mislav for recently devoting so much time to keeping this project afloat. I really like both ideas and I think you're right that most users would benefit from having a way to quickly mock out the interaction instead of wanting to verify the behavior so explicitly. Unfortunately I am spread incredibly thin at the moment so I don't know when/if I'll be able to implement this myself but maybe someone else will volunteer to take it on.

That being said, if you had the energy and time to try to work on it, I'd love to offer any help with debugging issues you are running into or providing any guidance you have. I just don't think I have the time to see it through from start to finish.

mislav commented 2 years ago

This is a great idea!

For Example: You want to test your newly written function askRandomQuestion asks the predefined question "Are you there?" and if the user responds with "Yes". If you're using go-expect, you first have to set up the environment, pass stdio and stderr to survey and than carefully check the complete text on the terminal, to make sure everything is working correctly.

As someone who struggled with making terminal interaction tests stable in even just this project, I would definitely not recommend that anyone spins up a terminal emulator in their own test suite just to test Survey interactions. Instead, some kind of mocking approach like you're suggesting would be much better, because in most cases you don't want to actually exercise Survey functionality within your own project's test suite. Hopefully, you trust that Survey is working as advertised.

In GitHub CLI we have our own mocking solution for Survey. It only suits our own needs, and its public interface isn't really suitable for other types of projects. If I find time, I can try polishing this up and moving it into this repository so it's reusable. Anyone is welcome to try beating me to that, though!

SirRegion commented 2 years ago

Hi @mislav,

thanks for your suggestion! I'll make a pull request adding a interface and a struct for survey itself.

SirRegion commented 2 years ago

Hi @mislav and @AlecAivazis, I added an interface for all the public functions in the survey.go file. I had a look at some of the other files, but I don't think they need to be part of the interface as they are not really needed in the mock.

I also added a a simple empty struct that implements all the functions by just calling them directly. We could also change the function signatures of all functions to directly include the struct, but this would be a breaking change.

I guess the pipeline should run without issues, but you first need to approve it. I'll be on vacation for the next two weeks, so I sadly can't contribute any code in that time, but I would be more than happy if you found the time to port the github mock solution to this repo @mislav!

See you soon!