Fix discrepancy between BasicUi and MockUi by moorara · Pull Request #71 · mitchellh/cli · GitHub
Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Fix discrepancy between BasicUi and MockUi#71

Merged
mitchellh merged 1 commit into
mitchellh:masterfrom
moorara:master
Mar 26, 2020
Merged

Fix discrepancy between BasicUi and MockUi#71
mitchellh merged 1 commit into
mitchellh:masterfrom
moorara:master

Conversation

@moorara

@moorara moorara commented Dec 25, 2017

Copy link
Copy Markdown
Contributor

I noticed the Ask method on BasicUi type uses bufio.Reader.ReadString() whereas the Ask method on MockUi uses fmt.Fscanln for reading from Reader input. This causes two problems:

When using BasicUi, one can enter a string with spaces. However, in tests when using MockUi, reading a string with spaces only returns the first word.
When using BasicUi, one can press Enter key and get a empty string back without any error, while using MockUi in tests returns an unexpected newline error.

This discrepancy between the actual implementation of Ask and the mocked implementation of Ask makes testing of the two cases mentioned above very hard.

This PR addresses this issue and make the actual and mocked implementation working the same way.

@mitchellh

Copy link
Copy Markdown
Owner

@mitchellh mitchellh merged commit 902dcca into mitchellh:master Mar 26, 2020
alisdair added a commit to hashicorp/terraform that referenced this pull request Sep 10, 2020
Update tests to match the fix in mitchellh/cli#71, which aligns MockUi
with BasicUi and allows newlines in user input.

We are not using the new ErrorWriter, added in mitchellh/cli#81, as it
does not appear to interact correctly with panicwrap. All error output
from CLI parsing will continue to appear on stdout, not stderr.
swalkinshaw added a commit to roots/trellis-cli that referenced this pull request Oct 30, 2020
This requires a newline after mitchellh/cli#71
swalkinshaw added a commit to roots/trellis-cli that referenced this pull request Oct 30, 2020
This requires a newline after mitchellh/cli#71
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants