#7: Implemented apt install command #24
Conversation
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
| assert "stderr" in result_str | ||
|
|
||
|
|
||
| def test_command_executor(monkeypatch, mock_command_result): |
There was a problem hiding this comment.
This test doesn't test the CommandExecutor because, you mocked its most important function. Part of this test could be used for testing CommandResult
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
sgn4sangar
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
|
|
||
|
|
||
| class CommandLogger(Protocol): | ||
| def info(self, msg: str) -> None: ... |
There was a problem hiding this comment.
| def info(self, msg: str) -> None: ... | |
| def info(self, msg: str, **kwargs) -> None: ... |
We probably should add to all methods the kwargs, then we can format the result if we want.
| self._last_msg = msg | ||
| sys.stderr.write(msg) | ||
|
|
||
| def get_last_msg(self) -> str: |
| class CommandResult: | ||
| def __init__( | ||
| self, | ||
| logger: CommandLogger, |
There was a problem hiding this comment.
I would move this to the end of the parameter list
| logger = StdLogger() | ||
| monkeypatch.setattr(CommandExecutor, "execute", mock_execute) |
There was a problem hiding this comment.
Why are you not using the Mock or MagicMock objects for Logger and CommandExecutor? This would remove the requirement for get_last_msg from the Logger which I don't like and the other test uses them as well.
| def mock_execute(_, cmd_strs): | ||
| return mock_command_result | ||
|
|
||
| def stdout_results(result_str: str): |
There was a problem hiding this comment.
If we have time left we should change this function to actually collect the strings in a list and assert in the end the strings. And, we probably should only use a single function, such that we see if the stdout and stderr are used alternatingly.
|
|
||
| @pytest.fixture | ||
| def mock_command_result(): | ||
| logger = StdLogger() |
There was a problem hiding this comment.
We should use a Mock object for this, such that we can check if print_results calls the Logger as expected.
| if len(apt_packages.packages) > 0: | ||
| update_cmd = prepare_update_command() | ||
| cmd_res = executor.execute(update_cmd) | ||
| cmd_res.print_results() |
There was a problem hiding this comment.
After we printed the results, we need to check the return_code if the command was successful. This is needed for each command.
fixes #7