Remove the workarounds for Click in testutils/runcli.py
- Dominant language
- Python
- Stars
- 140
- Forks
- 45
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 6
Description
[See original issue on GitLab](https://gitlab.com/BuildStream/buildstream/-/issues/729)
In GitLab by [[Gitlab user @gokcennurlu]](https://gitlab.com/gokcennurlu) on Oct 25, 2018, 15:39
## Background
While discussing [__How to catch regressions in BuildStream's interactive interface__ on the mailing list](https://mail.gnome.org/archives/buildstream-list/2018-October/msg00053.html), I had a look at Click's changes between it's 6.7 and 7.0 (released on 25th of September), and BuildStream's [tests/testutils/runcli.py](https://gitlab.com/BuildStream/buildstream/blob/master/tests/testutils/runcli.py).
When we were using Click 6.7, [this commit](https://gitlab.com/BuildStream/buildstream/commit/7397c7721c5815aaaf6f82c474f85e3e8359bb8e) added a feature (separation of stderr/stdout) to testutils that didn't exist in Click yet by using pytest's internals and mimicking CliRunner interface.
[Over time](https://gitlab.com/BuildStream/buildstream/commits/master/tests/testutils/runcli.py), more BuildStream related helper functions were added.
On September 25th, [solution to the initial issue](https://github.com/pallets/click/commit/fdceb39d344603fc73a4d9761766b5701f69236d#diff-637edab3f8e0763a9874d17879193dc9), the separation of stderr/stdout, was released as part of 7.0 and BuildStream started to use it since there is no version pinning for that at the moment.
I believe it is a good time to remove the workarounds that we don't need them. By removing/simplifying `tests.testutils.runcli.Result` and simplifying the functions in `tests.testutils.runcli.Cli` we can remove lots of maintenance burden.
Finally, I haven't spent too much time on it but, this refactor can also help with the original issue (testing interactive parts of BuildStream) in the mailing list.
## Task description
[//]: # (Short summary of the action to be executed)
* [ ] Simplify or remove `tests.testutils.runcli.Result`
* [ ] Remove the need for `from _pytest.capture import MultiCapture, FDCapture`
* [ ] Clean up `tests.testutils.runcli.Cli.invoke()`, consider extending `click.testing.CliRunner` for `tests.testutils.runcli.Cli`
## Acceptance Criteria
[//]: # (Acceptance criteria should follow the S.M.A.R.T. principle https://en.wikipedia.org/wiki/SMART_criteria )
`tests.testutils.runcli.py` is free from workarounds are not necessary anymore.
----
Contributor guide
Research direction
Start in tests/testutils/runcli.py, focusing on tests.testutils.runcli.Result, Cli, and Cli.invoke(), and compare their behavior with Click 7's click.testing.CliRunner. Check where MultiCapture and FDCapture are imported and used. Done means the file no longer relies on unnecessary Click workarounds while preserving the existing test utilities' behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100