Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: fix missing mock in attach unit test #2785

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 23 additions & 38 deletions uaclient/cli/tests/test_cli_attach.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from uaclient import event_logger, http, messages, util
from uaclient.cli import (
_post_cli_attach,
action_attach,
attach_parser,
get_parser,
Expand Down Expand Up @@ -231,6 +232,25 @@ def test_lock_file_exists(
}
assert expected == json.loads(capsys.readouterr()[0])

@mock.patch(
"uaclient.status.format_tabular", return_value="mock_tabular_status"
)
@mock.patch("uaclient.actions.status", return_value=("", 0))
@mock.patch(M_PATH + "daemon")
def test_post_cli_attach(
self, m_daemon, m_status, m_format_tabular, capsys, FakeConfig
):
cfg = FakeConfig.for_attached_machine()
_post_cli_attach(cfg)

assert [mock.call()] == m_daemon.stop.call_args_list
assert [mock.call(cfg)] == m_daemon.cleanup.call_args_list
assert [mock.call(cfg)] == m_status.call_args_list
assert [mock.call("")] == m_format_tabular.call_args_list
out, _ = capsys.readouterr()
assert "This machine is now attached to 'test_contract'" in out
assert "mock_tabular_status" in out

@mock.patch(
M_PATH + "contract.UAContractClient.update_activity_token",
)
Expand All @@ -239,12 +259,10 @@ def test_lock_file_exists(
@mock.patch("uaclient.files.notices.NoticesManager.remove")
@mock.patch("uaclient.timer.update_messaging.update_motd_messages")
@mock.patch(M_PATH + "contract.UAContractClient.add_contract_machine")
@mock.patch("uaclient.actions.status", return_value=("", 0))
@mock.patch("uaclient.status.format_tabular")
@mock.patch(M_PATH + "_post_cli_attach")
def test_happy_path_with_token_arg(
self,
m_format_tabular,
m_status,
m_post_cli,
contract_machine_attach,
m_update_apt_and_motd_msgs,
_m_remove_notice,
Expand All @@ -270,46 +288,13 @@ def fake_contract_attach(contract_token, attachment_dt):
ret = action_attach(args, cfg)

assert 0 == ret
assert 1 == m_status.call_count
assert 1 == m_format_tabular.call_count
expected_calls = [
mock.call(contract_token=token, attachment_dt=mock.ANY)
]
assert expected_calls == contract_machine_attach.call_args_list
assert [mock.call(cfg)] == m_update_apt_and_motd_msgs.call_args_list
assert 1 == m_update_activity_token.call_count

# We need to do that since all config objects in this
# test will share the same data dir. Since this will
# test a successful attach, in the end we write a machine token
# file, which will make all other cfg objects here to report
# as attached
cfg.delete_cache()
cfg.machine_token_file.delete()

cfg = FakeConfig()
args = mock.MagicMock(token=token, attach_config=None)
with mock.patch.object(
event, "_event_logger_mode", event_logger.EventLoggerMode.JSON
):
with mock.patch.object(
cfg, "check_lock_info"
) as m_check_lock_info:
m_check_lock_info.return_value = (0, "lock_holder")
fake_stdout = io.StringIO()
with contextlib.redirect_stdout(fake_stdout):
main_error_handler(action_attach)(args, cfg)

expected = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this new setup we are no longer testing the event_logger part of the command. Should we add an explicit test on for this as well ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure - there are tests in tests/event_logger.py to cover that, and the text output covers the infos.
I would even go further and mock the event logger to be honest - the unit test should not be responsible for that after all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have integration tests that cover json output for attach, so I think this is okay to remove.

"_schema_version": event_logger.JSON_SCHEMA_VERSION,
"result": "success",
"errors": [],
"failed_services": [],
"needs_reboot": False,
"processed_services": [],
"warnings": [],
}
assert expected == json.loads(fake_stdout.getvalue())
assert [mock.call(cfg)] == m_post_cli.call_args_list

@pytest.mark.parametrize("auto_enable", (True, False))
@mock.patch(
Expand Down
Loading