Remove the workarounds for Click in testutils/runcli.py #729

Open BuildStream-Migration-Bot opened 3 years ago

See original issue on GitLab In GitLab by [Gitlab user @gokcennurlu] on Oct 25, 2018, 15:39

Background

While discussing How to catch regressions in BuildStream's interactive interface on the mailing list, 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.

When we were using Click 6.7, this commit 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, more BuildStream related helper functions were added.

On September 25th, solution to the initial issue, 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

[ ] 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

tests.testutils.runcli.py is free from workarounds are not necessary anymore.

In GitLab by [Gitlab user @gokcennurlu] on Oct 25, 2018, 15:47

changed the description

apache / buildstream