-
Notifications
You must be signed in to change notification settings - Fork 355
Added an interface for all the public functions of the survey package #429
base: master
Are you sure you want to change the base?
Changes from 7 commits
cc375fe
dc306a6
5e8fe73
155eb18
ed80995
96b15b2
80f6737
a462a5c
b9c3cb5
5b78430
69f3ff0
5ba9322
79a6d75
fe82ce8
f62075a
d70c6af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -442,6 +442,10 @@ survey.AskOne( | |
|
||
## Testing | ||
|
||
There are two ways to test a program using survey. Using a mock is recommended because it leads to faster, less brittle tests. | ||
|
||
### Using a simulated terminal | ||
|
||
You can test your program's interactive prompts using [go-expect](https://github.com/Netflix/go-expect). The library | ||
can be used to expect a match on stdout and respond on stdin. Since `os.Stdout` in a `go test` process is not a TTY, | ||
if you are manipulating the cursor or using `survey`, you will need a way to interpret terminal / ANSI escape sequences | ||
|
@@ -450,6 +454,65 @@ stdio to an in-memory [virtual terminal](https://github.com/hinshun/vt10x). | |
|
||
For some examples, you can see any of the tests in this repo. | ||
|
||
### Using a mock | ||
|
||
Instead of calling the survey functions directly, you can create a survey struct and call the functions from there. | ||
|
||
```golang | ||
|
||
survey := survey.Survey{} | ||
|
||
response := false | ||
prompt := &survey.Confirm{ | ||
Message: "Do you like pie?", | ||
} | ||
survey.AskOne(prompt, &response) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now the big question is whether we should amend all other examples in the Readme to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess updating the readme should not be to much work, as it's just one or two lines of code changed in each example. I left it the same for this pull request, because I wasn't sure how much of a "breaking" change you would accept for the docs. If we decide to update the docs, I would take on the work! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the examples in the readme, but I did not touch the text for now. Some parts ("Running the Prompts", "Testing") likely still need updating. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, good question. I think keeping the examples as short as possible is probably the best. I'd like to avoid the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think explicitly using Referencing an undefined I would recommend updating the first example to include a function where the surveyor is passed as an argument to show how it is done "correctly". |
||
|
||
``` | ||
|
||
If you create only one survey struct at the top level of your program and pass it to all functions, you can test those functions by replacing the struct with a mock provided by survey. | ||
|
||
#### main | ||
|
||
```golang | ||
func main() { | ||
survey := survey.Survey{} | ||
|
||
AskForPie(survey) | ||
} | ||
|
||
func AskForPie(survey survey.SurveyInterface) bool { | ||
response := false | ||
prompt := &survey.Confirm{ | ||
Message: "Do you like pie?", | ||
} | ||
survey.AskOne(prompt, &response) | ||
|
||
return response | ||
} | ||
|
||
``` | ||
|
||
#### Test | ||
|
||
```golang | ||
func TestAskForPie(t *testing.T) { | ||
|
||
//create mock | ||
mock := survey.SurveyMock{} | ||
//set the response the "user" should select | ||
mock.setResponse(true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If AskForPie is going to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation does not allow to set multiple responses and would always use the last one that is set. Alternatively, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel strongly that the mock solution should be flexible enough to mock all responses, no matter how many times Ask or AskOne has been called. I may be able to contribute this improvement to your mock implementation. Example use case: surveyer := survey.SurveyMock{}
surveyer.SetReponse(map[string]interface{}{
"name": "foo",
"message": "hello",
})
DoSomething(surveyer) This test setup will work as long as DoSomething is calling Ask with two questions. However, let's say that the implementation of DoSomething needs to change because the prompt of question 2 needs to dynamically adjust to the response from question 1. The way to do that with survey is to switch from Ask to 2x AskOne. After the implementation changes in this way, the current test from the example will break. However, if the mock solution does not support stubbing several AskOnes, then the user will be forced to break up DoSomething into separate functions just to appease the SurveyMock implementation. I think this would cause frustration to the user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a usecase i did not think of before. How would you feel about something like this (inspired by jest): mock := survey.Surveyor{}
mock.SetResponseOnce("first").SetResponseOnce("second").SetRespose("all others")
DoSomething(surveyor) Now the mock would respond to the first question inside Now if If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like your proposal; it looks more flexible already! One thing that I still think would be great if SurveyMock could do is verify that the prompt was shown to the user with the right text, since this text is the most important part of how the user interacts with Survey. We do it like this, which feels similar to what you've proposed, but allows for a greater level of precision. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if we're going down this path we probably also want to provide methods on the mock for setting a specific field with a value (something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SirRegion Yes, let's undo the overall README changes for now (to keep things as they were before) and have there just be a small mention & example of the mock functionality. @AlecAivazis How would the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean to have both methods so a user could decide if they want to test the order of responses or just if a response was given for a field. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AlecAivazis Agreed! It's probably better to be strict with verification out-of-the-box (i.e. to verify the order of prompts by default) but give the developer a chance to opt out if needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mislav I reverted the commit that changed the readme for now and updated the Testing part to include a warning about the unstable mock API. |
||
|
||
result := AskForPie(mock) | ||
|
||
//check output of the function | ||
if result != true { | ||
SirRegion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
t.Fatalf("AskForPie returned false, but it should have returned true!") | ||
SirRegion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
``` | ||
|
||
## FAQ | ||
|
||
### What kinds of IO are supported by `survey`? | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package survey | ||
|
||
import ( | ||
"io" | ||
|
||
"github.com/AlecAivazis/survey/v2/core" | ||
"github.com/AlecAivazis/survey/v2/terminal" | ||
) | ||
|
||
type SurveyMock struct { | ||
singleResponse interface{} | ||
multipleResponses map[string]interface{} | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithStdio(in terminal.FileReader, out terminal.FileWriter, err io.Writer) AskOpt { | ||
SirRegion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithFilter(filter func(filter string, value string, index int) (include bool)) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithKeepFilter(KeepFilter bool) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithValidator(v Validator) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithPageSize(pageSize int) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithHelpInput(r rune) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithIcons(setIcons func(*IconSet)) AskOpt { | ||
return nil | ||
} | ||
|
||
// Not implemented, because it does not affect the mock | ||
func (mock *SurveyMock) WithShowCursor(ShowCursor bool) AskOpt { | ||
return nil | ||
} | ||
|
||
func (mock *SurveyMock) AskOne(p Prompt, response interface{}, opts ...AskOpt) error { | ||
err := core.WriteAnswer(response, "", mock.singleResponse) | ||
if err != nil { | ||
// panicing is fine inside a mock | ||
panic(err) | ||
} | ||
return nil | ||
} | ||
|
||
func (mock *SurveyMock) Ask(qs []*Question, response interface{}, opts ...AskOpt) error { | ||
for _, q := range qs { | ||
|
||
err := core.WriteAnswer(response, q.Name, mock.multipleResponses[q.Name]) | ||
if err != nil { | ||
// panicing is fine inside a mock | ||
panic(err) | ||
} | ||
|
||
} | ||
|
||
return nil | ||
} | ||
|
||
func (mock *SurveyMock) SetResponse(response interface{}) { | ||
if val, ok := response.(map[string]interface{}); ok { | ||
mock.multipleResponses = val | ||
} else { | ||
mock.singleResponse = response | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package survey | ||
|
||
import ( | ||
"testing" | ||
) | ||
|
||
func TestMockAskOne(t *testing.T) { | ||
t.Run("Setting Response", func(t *testing.T) { | ||
mock := SurveyMock{} | ||
mock.SetResponse(true) | ||
|
||
prompt := &Confirm{ | ||
Message: "test", | ||
} | ||
|
||
var response bool | ||
mock.AskOne(prompt, &response) | ||
|
||
if !response { | ||
t.Fatalf("Response was false but should have been true!") | ||
} | ||
}) | ||
|
||
} | ||
|
||
func TestMockAsk(t *testing.T) { | ||
t.Run("Setting Response", func(t *testing.T) { | ||
mock := SurveyMock{} | ||
|
||
test := make(map[string]interface{}) | ||
test["test"] = true | ||
|
||
mock.SetResponse(test) | ||
|
||
questions := []*Question{ | ||
{ | ||
Name: "test", | ||
Prompt: &Confirm{ | ||
Message: "testing", | ||
}, | ||
}, | ||
} | ||
|
||
answer := struct { | ||
Test bool | ||
}{} | ||
mock.Ask(questions, &answer) | ||
|
||
if !answer.Test { | ||
t.Fatalf("Response was false but should have been true!") | ||
} | ||
}) | ||
} |
Uh oh!
There was an error while loading. Please reload this page.