Skip to content

Commit

Permalink
Updated unit tests and behave tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@dheyay
dheyay committed Feb 14, 2024
1 parent 4cc1fc0 commit 7bc022e
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 99 deletions.
36 changes: 12 additions & 24 deletions features/unattached_status.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,35 +602,23 @@ Feature: Unattached status

Scenario Outline: Check notice file read permission
Given a `<release>` `<machine_type>` machine with ubuntu-advantage-tools installed
When I run `rm -rf /run/ubuntu-advantage` with sudo
When I run `mkdir -p /run/ubuntu-advantage/notices` with sudo
When I run `touch /run/ubuntu-advantage/notices/crasher` with sudo
When I run `chmod 0 /run/ubuntu-advantage/notices/crasher` with sudo
When I run `rm -rf /var/ubuntu-advantage` with sudo
When I run `mkdir -p /var/ubuntu-advantage/notices` with sudo
When I run `touch /var/ubuntu-advantage/notices/crasher` with sudo
When I run `chmod 0 /var/ubuntu-advantage/notices/crasher` with sudo
When I run `mkdir -p /var/lib/ubuntu-advantage/notices` with sudo
When I run `touch /var/lib/ubuntu-advantage/notices/crasher` with sudo
When I run `chmod 0 /var/lib/ubuntu-advantage/notices/crasher` with sudo
When I run `touch /var/lib/ubuntu-advantage/notices/10-reboot_required` with sudo
When I run `pro status` as non-root
Then stdout matches regexp:
"""
SERVICE +AVAILABLE +DESCRIPTION
(anbox-cloud +(yes|no) +.*)?
?cc-eal +yes +Common Criteria EAL2 Provisioning Packages
cis +yes +Security compliance and audit tools
esm-apps +yes +Expanded Security Maintenance for Applications
esm-infra +yes +Expanded Security Maintenance for Infrastructure
fips +yes +NIST-certified FIPS crypto packages
fips-updates +yes +FIPS compliant crypto packages with stable security updates
livepatch +yes +(Canonical Livepatch service|Current kernel is not supported)
ros +yes +Security Updates for the Robot Operating System
ros-updates +yes +All Updates for the Robot Operating System
For a list of all Ubuntu Pro services, run 'pro status --all'
This machine is not attached to an Ubuntu Pro subscription.
See https://ubuntu.com/pro
NOTICES
System reboot required
"""
Examples: ubuntu release
| release | machine_type |
| xenial | lxd-container |
| bionic | lxd-container |
| release | machine_type |
| xenial | lxd-container |
| bionic | lxd-container |
| focal | lxd-container |
| jammy | lxd-container |
| mantic | lxd-container |
4 changes: 2 additions & 2 deletions uaclient/cli/tests/test_cli_status.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,8 @@ def test_status_help(
([], ""),
(
[
[FakeNotice.a, "adesc"],
[FakeNotice.b, "bdesc"],
[FakeNotice.reboot_required, "adesc"],
[FakeNotice.enable_reboot_required, "bdesc"],
],
"\nNOTICES\nadesc\nbdesc\n",
),
Expand Down
12 changes: 9 additions & 3 deletions uaclient/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,15 @@ def event():


class FakeNotice(NoticeFileDetails, Enum):
a = NoticeFileDetails("01", "a", True, "notice_a")
a2 = NoticeFileDetails("03", "a2", True, "notice_a2")
b = NoticeFileDetails("02", "b2", False, "notice_b")
reboot_required = NoticeFileDetails(
"10", "reboot_required", False, "notice_a"
)
reboot_script_failed = NoticeFileDetails(
"12", "reboot_script_failed", True, "notice_a2"
)
enable_reboot_required = NoticeFileDetails(
"11", "enable_reboot_required", False, "notice_b"
)


@pytest.yield_fixture(autouse=True)
Expand Down
55 changes: 32 additions & 23 deletions uaclient/files/notices.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,38 +170,49 @@ def _get_notice_file_names(self, directory: str) -> List[str]:
file_name
for file_name in os.listdir(directory)
if os.path.isfile(os.path.join(directory, file_name))
and self._is_valid_notice_file(directory, file_name)
]

def _is_readable(self, directory: str, file_name: str) -> bool:
"""Checks if the notice file is readable.
def _is_valid_notice_file(self, directory: str, file_name: str) -> bool:
"""Checks if the notice file is valid.
:param directory: The directory where the notice file is located.
:param file_name: The name of the notice file.
:returns: True if the file is readable, False otherwise.
:returns: True if the file is valid, False otherwise.
"""
return os.access(os.path.join(directory, file_name), os.R_OK)
try:
order_id, label = file_name.split("-")
for n in Notice:
if (
n.order_id == order_id
and n.label == label
and (
(
directory == defaults.NOTICES_PERMANENT_DIRECTORY
and n.is_permanent
)
or (
directory == defaults.NOTICES_TEMPORARY_DIRECTORY
and not n.is_permanent
)
)
):
return True
return False
except ValueError:
return False

def _get_default_message(self, file_name: str) -> str:
"""Gets the default message for a notice file.
:param file_name: The name of the notice file.
:returns: The default message defined in the enum.
"""
try:
order_id, label = file_name.split("-")
notice = None
for n in Notice:
if n.order_id == order_id and n.label == label:
notice = n
if notice is None:
raise Exception()
return notice.value.message
except Exception:
LOG.warning(
"Something went wrong while processing notice: %s.",
file_name,
)
return ""
order_id, label = file_name.split("-")
notice = None
for notice in Notice:
if notice.order_id == order_id and notice.label == label:
return notice.value.message
return ""

def list(self) -> List[str]:
"""Gets all the notice files currently saved.
Expand All @@ -218,15 +229,13 @@ def list(self) -> List[str]:
continue
notice_file_names = self._get_notice_file_names(notice_directory)
for notice_file_name in notice_file_names:
if not self._is_readable(notice_directory, notice_file_name):
continue
try:
notice_file_contents = system.load_file(
os.path.join(notice_directory, notice_file_name)
)
except PermissionError:
LOG.warning(
f"Permission error while reading {notice_file_name}"
"Permission error while reading " + notice_file_name
)
continue
if notice_file_contents:
Expand Down
57 changes: 29 additions & 28 deletions uaclient/files/tests/test_notices.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ class TestNotices:
"label,content",
(
(
FakeNotice.a,
"notice_a",
FakeNotice.reboot_script_failed,
"notice_a2",
),
),
)
Expand All @@ -30,7 +30,10 @@ def test_add(
notice.add(label, content)
assert [
mock.call(
os.path.join(defaults.NOTICES_PERMANENT_DIRECTORY, "01-a"),
os.path.join(
defaults.NOTICES_PERMANENT_DIRECTORY,
"12-reboot_script_failed",
),
content,
)
] == sys_write_file.call_args_list
Expand All @@ -44,15 +47,18 @@ def test_add_non_root(
caplog_text,
):
notice = NoticesManager()
notice.add(FakeNotice.a, "content")
notice.add(FakeNotice.reboot_required, "content")
assert [] == m_sys_write_file.call_args_list
assert "NoticesManager.add(a) called as non-root user" in caplog_text()
assert (
"NoticesManager.add(reboot_required) called as non-root user"
in caplog_text()
)

@pytest.mark.parametrize(
"label,content",
(
(
FakeNotice.a,
FakeNotice.reboot_required,
"notice_a",
),
),
Expand All @@ -74,8 +80,8 @@ def test_add_duplicate_label(
"label,content",
(
(
FakeNotice.a,
"notice_a",
FakeNotice.reboot_script_failed,
"notice_a2",
),
),
)
Expand All @@ -91,7 +97,10 @@ def test_remove(
notice.remove(label)
assert [
mock.call(
os.path.join(defaults.NOTICES_PERMANENT_DIRECTORY, "01-a"),
os.path.join(
defaults.NOTICES_PERMANENT_DIRECTORY,
"12-reboot_script_failed",
),
)
] == sys_file_absent.call_args_list

Expand All @@ -104,10 +113,11 @@ def test_remove_non_root(
caplog_text,
):
notice = NoticesManager()
notice.remove(FakeNotice.a)
notice.remove(FakeNotice.reboot_required)
assert [] == m_sys_file_absent.call_args_list
assert (
"NoticesManager.remove(a) called as non-root user" in caplog_text()
"NoticesManager.remove(reboot_required) called as non-root user"
in caplog_text()
)

@mock.patch("uaclient.files.notices.NoticesManager.list")
Expand All @@ -116,12 +126,14 @@ def test_remove_non_root(
def test_notice_module(
self, notice_cls_add, notice_cls_remove, notice_cls_read
):
notices.add(FakeNotice.a)
notices.add(FakeNotice.reboot_required)
assert [
mock.call(FakeNotice.a, "notice_a"),
mock.call(FakeNotice.reboot_required, "notice_a"),
] == notice_cls_add.call_args_list
notices.remove(FakeNotice.a)
assert [mock.call(FakeNotice.a)] == notice_cls_remove.call_args_list
notices.remove(FakeNotice.reboot_required)
assert [
mock.call(FakeNotice.reboot_required)
] == notice_cls_remove.call_args_list
notices.list()
assert 1 == notice_cls_read.call_count

Expand All @@ -135,27 +147,16 @@ def test_get_notice_file_names(self, m_get_notice_file_names):
"directory"
)

@mock.patch("uaclient.files.notices.os.access")
def test_is_readable(self, m_access):
notice = NoticesManager()
m_access.return_value = False
assert not notice._is_readable("directory", "file")
assert [] == notice.list()
m_access.assert_called_once_with("directory/file", os.R_OK)

@mock.patch("uaclient.files.notices.NoticesManager._get_notice_file_names")
@mock.patch("uaclient.files.notices.NoticesManager._is_readable")
@mock.patch("uaclient.system.load_file")
def test_list(self, m_load_file, m_is_readable, m_get_notice_file_names):
def test_list(self, m_load_file, m_get_notice_file_names):
notice = NoticesManager()

m_get_notice_file_names.side_effect = (
lambda directory: []
if directory == defaults.NOTICES_TEMPORARY_DIRECTORY
else ["fakeNotice1", "fakeNotice2"]
else ["fakeNotice1"]
)
m_is_readable.side_effect = [True, False]
m_load_file.return_value = "test"

assert ["test"] == notice.list()
assert 2 == notice._is_readable.call_count
38 changes: 19 additions & 19 deletions uaclient/tests/test_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ class TestNotices:
(
([], ()),
(
[[FakeNotice.a2, "a1"]],
[[FakeNotice.reboot_script_failed, "a1"]],
["a1"],
),
(
[
[FakeNotice.a, "a1"],
[FakeNotice.a2, "a2"],
[FakeNotice.reboot_required, "a1"],
[FakeNotice.reboot_script_failed, "a2"],
],
[
"a1",
Expand All @@ -89,8 +89,8 @@ class TestNotices:
),
(
[
[FakeNotice.a, "a1"],
[FakeNotice.a, "a1"],
[FakeNotice.reboot_required, "a1"],
[FakeNotice.reboot_required, "a1"],
],
[
"a1",
Expand All @@ -116,11 +116,11 @@ def test_add_notice_avoids_duplicates(
"_notices",
(
([]),
([[FakeNotice.a]]),
([[FakeNotice.reboot_required]]),
(
[
[FakeNotice.a],
[FakeNotice.a2],
[FakeNotice.reboot_required],
[FakeNotice.reboot_script_failed],
]
),
),
Expand All @@ -139,29 +139,29 @@ def test_add_notice_fails_as_nonroot(
@pytest.mark.parametrize(
"notices_,removes,expected",
(
([], [FakeNotice.a], []),
([], [FakeNotice.reboot_required], []),
(
[[FakeNotice.a2]],
[FakeNotice.a2],
[[FakeNotice.reboot_script_failed]],
[FakeNotice.reboot_script_failed],
[],
),
(
[
[FakeNotice.a],
[FakeNotice.a2],
[FakeNotice.reboot_required],
[FakeNotice.reboot_script_failed],
],
[FakeNotice.a],
[FakeNotice.reboot_required],
["notice_a2"],
),
(
[
[FakeNotice.a],
[FakeNotice.a2],
[FakeNotice.b],
[FakeNotice.reboot_required],
[FakeNotice.reboot_script_failed],
[FakeNotice.enable_reboot_required],
],
[
FakeNotice.a,
FakeNotice.a2,
FakeNotice.reboot_required,
FakeNotice.reboot_script_failed,
],
["notice_b"],
),
Expand Down

0 comments on commit 7bc022e

Please sign in to comment.