Decide on whether to keep `launch_command` or switch to `os.popen` to run git machete commands in tests

[BartoszSiwinski]

Currently (after #320 is merged) there are 2 approaches to running git machete commands in tests:

1. using MacheteTester.launch_command()
2. using os.popen() (as context manager).

The latter one was introduced, because if some commands were opening less it was not possible to read the output using launch_command approach.

This issue goal is to unify the approach to running git machete commands in tests by either updating launch_command so it can read all the outputs or by using os.popen everywhere.

[PawelLipski]

See the above mention (https://github.com/VirtusLab/git-machete/pull/320#discussion_r737592249) for discussion. There might be only launch_command() across all the tests, but still migrating to os.popen might be a good idea anyway.

[PawelLipski]

@BartoszSiwinski pls update the issue description, it might no longer be valid

[BartoszSiwinski]

It's seems up-to-date. We still want to find a common approach. There might be just more things to consider based on comments in mentioned PRs while choosing the solution.

[PawelLipski]

os.popen will suck wrt. mocking :/ so e.g. testing git machete github won't be possible

[AABur]

I analysed tests. Currently os.popen is used to:

• creating a temporary directory popen("mktemp -d")
• calling git commands

popen("mktemp -d") can be replaced using pytest tmp_path fixture

for git commands, using machete launch_command is definitely not acceptable

[AABur]

This issue can be closed. Only launch_command is used to run git machete commands in tests It may be useful to mention it in the contribution guide for future tests development.

[PawelLipski]

Okay! if you could piggyback that change to CONTRIBUTING.md onto your PR #515

[AABur]

Okay! if you could piggyback that change to CONTRIBUTING.md onto your PR #515

I'd instead do it separately. Two words are not enough here. In writing a contributing guide, it is important to write it well.