Raise an exception on end of input in CliRunner#2934
Merged
davidism merged 1 commit intopallets:stablefrom May 20, 2025
Merged
Conversation
kieranyyu
commented
May 20, 2025
| click.echo(f.name) | ||
|
|
||
| result = runner.invoke(cli) | ||
| result = runner.invoke(cli, input="\n") |
Contributor
Author
There was a problem hiding this comment.
Newline needs to be inputted to simulate user input
Member
|
I think we do want to raise try:
next()
except StopIteration:
raise EOFError |
kieranyyu
commented
May 20, 2025
| def mode(self) -> str: | ||
| return self._mode | ||
|
|
||
| def __next__(self) -> str: # type: ignore |
Contributor
Author
There was a problem hiding this comment.
mypy claims the return type should be bytes because of IOBase but it should be matching with TextIOWrapper
Same issue as python/mypy#9643
e8fd432 to
e4f590e
Compare
26ce633 to
80efdf6
Compare
asottile-sentry
added a commit
to getsentry/sentry
that referenced
this pull request
May 21, 2025
1 task
timoffex
pushed a commit
to wandb/wandb
that referenced
this pull request
May 22, 2025
Description ----------- <!-- Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable) --> What does the PR do? Include a concise description of the PR contents. This PR fixes consistent test failures related to `wand docker` cli tests. This was caused by an update with our dependency `click` in version `v8.2.1`, which changed the behavior with how the `CliRunner` is handling input when there is a prompt but no more input provided. (see click [issue here](pallets/click#2934), and the [fix here](pallets/click#2934)) <!-- NEW: We're using a new changelog format that's more useful for users. Please see CHANGELOG.unreleased.md for details and update on relevant changes such as feature additions, bug fixes, or removals/deprecations. --> - [ ] I updated CHANGELOG.unreleased.md, or it's not applicable Testing ------- How was this PR tested? - running unit tests for `tests/unit_tests/test_cli.py` <!-- Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits) -->
dmitryduev
pushed a commit
to wandb/wandb
that referenced
this pull request
May 22, 2025
Description ----------- <!-- Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable) --> What does the PR do? Include a concise description of the PR contents. This PR fixes consistent test failures related to `wand docker` cli tests. This was caused by an update with our dependency `click` in version `v8.2.1`, which changed the behavior with how the `CliRunner` is handling input when there is a prompt but no more input provided. (see click [issue here](pallets/click#2934), and the [fix here](pallets/click#2934)) <!-- NEW: We're using a new changelog format that's more useful for users. Please see CHANGELOG.unreleased.md for details and update on relevant changes such as feature additions, bug fixes, or removals/deprecations. --> - [ ] I updated CHANGELOG.unreleased.md, or it's not applicable Testing ------- How was this PR tested? - running unit tests for `tests/unit_tests/test_cli.py` <!-- Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits) -->
the-13th-letter
added a commit
to the-13th-letter/derivepassphrase
that referenced
this pull request
May 29, 2025
A discrepancy exists between the documentation of `click.prompt` and the actual behavior of `click.prompt` when mocked with `click.testing.CliRunner` on click 8.2.0 and below ([`pallets/click#2934`][BUG2934]): when prompting for input and if at end-of-file, the `CliRunner` may return an empty string instead of raising `click.Abort`. This usually translates to extra line breaks in the "mixed" and "echoed" runner output at the last prompt(s). We mitigate this discrepancy from both sides. On the code side, we wrap each call to `click.prompt` to treat aborts and empty responses the same, which is appropriate behavior for the types of prompts we issue. On the test side, we amend our existing tests to use empty-line input instead of no input, and explicitly test the "no input" scenario separately, accepting both the 8.2.0-or-lower or the 8.2.1-or-higher output. Because the behavior depends on the `click` version, which is beyond our control, we also adjust coverage measurement. [BUG2934]: pallets/click#2934
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When end of input is reached in CliRunner,
_NamedTextIOWrapper.readlinecontinues to return an empty string which does not match the behavior when running the program at the command line (in this case an EOFError is raised which causes an Abort to be raised).Using
next()instead ofreadlinewill raiseStopIterationfromTextIOWrapper. If we want to perfectly match the behavior of regular input, we could override__next__to raiseEOFError(this will require a type: ignore due to mypy comparing types withIOBaseinstead ofTextIOWrapper).fixes #2787