Conversation
Both SurveyAsk and SurveyAskOne methods now share the same sets of
stubs, making it possible to change which of these methods is used in
the implementation without breaking tests.
A new method `AskStubber.StubPrompt("<prompt>")` is added as test helper
to supersede old Stub and StubOne methods. The new helper matches on
prompt messages rather than on field names, enabling tests to be written
based on what the user would see rather than coupling to implementation
details.
The new stubber also allows verifying whether a Select or MultiSelect
was rendered with the expected set of options. Furthermore, if a stubbed
value is not present among those options, the stubber will panic instead
of continuing normally.
Stubbed Selects with an int instead of a string target receiver are now
transparently handled. The values for Select stubs are always strings in
tests, but the stubber will write an int answer if the receiver expects
one as a selected index instead of a selected string value.
Lastly, this set of changes improves test resiliency since the stubs are
now matched based on prompt message (or field name for legacy stubs
created with Stub) instead of sequentially, enabling the implementation
to reorder the prompts without breaking existing tests.
samcoe
reviewed
Jan 14, 2022
Contributor
samcoe
left a comment
There was a problem hiding this comment.
This is sooo much nicer than before! I love it. Left just a couple questions. Also looks like the linter is not passing, was the plan to fix all those before merging or overtime making those changes?
| panic(fmt.Sprintf("more asks than stubs. most recent call: %#v", qs)) | ||
| var stub *QuestionStub | ||
| for _, s := range as.stubs { | ||
| if !s.matched && (s.message == "" && strings.EqualFold(s.Name, fieldName) || s.message == message) { |
Contributor
There was a problem hiding this comment.
Supporting regex matching on the message could be a nice improvement for the future, not necessary to add now.
Contributor
Author
|
@samcoe I've ported some more legacy stubs, but there are many more remaining, so I've allow-listed remaining ones with |
samcoe
approved these changes
Jan 17, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both SurveyAsk and SurveyAskOne methods now share the same sets of stubs, making it possible to change which of these methods is used in the implementation without breaking tests.
A new method
AskStubber.StubPrompt("<prompt>")is added as test helper to supersede old Stub and StubOne methods. The new helper matches on prompt messages rather than on field names, enabling tests to be written based on what the user would see rather than coupling to implementation details.The new stubber also allows verifying whether a Select or MultiSelect was rendered with the expected set of options. Furthermore, if a stubbed value is not present among those options, the stubber will panic instead of continuing normally.
Stubbed Selects with an
intinstead of astringtarget receiver are now transparently handled. The values for Select stubs are always strings in tests, but the stubber will write anintanswer if the receiver expects one as a selected index instead of a selected string value.Lastly, this set of changes improves test resiliency since the stubs are now matched based on prompt message (or field name for legacy stubs created with Stub) instead of sequentially, enabling the implementation to reorder the prompts without breaking existing tests.
Because the new stubbing API is no longer concerned with Survey internals, this allows us to potentially transition off of Survey in the future but keep the test stubs.
Some before vs. after: