Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Feature Request: Survey Mock #428

Open
SirRegion opened this issue May 31, 2022 · 4 comments
Open

Feature Request: Survey Mock #428

SirRegion opened this issue May 31, 2022 · 4 comments
Labels

Comments

@SirRegion
Copy link
Contributor

SirRegion commented May 31, 2022

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
Copy link
Owner

AlecAivazis commented Jun 2, 2022

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
Copy link
Collaborator

mislav commented Jun 2, 2022

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
Copy link
Contributor Author

Hi @mislav,

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

@SirRegion
Copy link
Contributor Author

SirRegion commented Jun 2, 2022

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants