added orndorffgrant's suggestions from PR

uaclient/cli/tests/test_cli.py

@@ -511,7 +511,6 @@ def test_errors_handled_gracefully(
         m_log_exception,
         m_event_info,
         m_delete_cache_key,
-        # _m_get_user_or_root_log_path,
         event,
         exception,
         expected_error_msg,

uaclient/cli/tests/test_cli_attach.py

@@ -537,7 +537,6 @@ def test_attach_when_one_service_fails_to_enable(
         m_process_entitlement_delta,
         m_enable_order,
         _m_attachment_data_file_write,
-        # _m_get_user_or_root_log_path,
         expected_exception,
         expected_msg,
         expected_outer_msg,

uaclient/log.py

@@ -70,7 +70,7 @@ def get_user_or_root_log_file_path() -> str:
     if util.we_are_currently_root():
         return UAConfig().log_file
     else:
-        return "~/.cache/" + defaults.USER_CACHE_SUBDIR + "/ubuntu-pro.log"
+        return get_user_log_file()
 
 
 def get_user_log_file() -> str:

uaclient/tests/test_log.py

@@ -162,39 +162,46 @@ def test_valid_json_output(
 
 
 class TestLogHelpers:
-    def test_get_user_or_root_log_file_path(self):
+    @pytest.mark.parametrize(
+        [
+            "we_are_currently_root",
+            "expected",
+        ],
+        [
+            (True, "cfg_log_file"),
+            (False, "user_log_file"),
+        ],
+    )
+    @mock.patch(
+        "uaclient.log.get_user_log_file",
+        return_value="user_log_file",
+    )
+    @mock.patch(
+        "uaclient.config.UAConfig.log_file",
+        new_callable=mock.PropertyMock,
+        return_value="cfg_log_file",
+    )
+    @mock.patch("uaclient.util.we_are_currently_root")
+    def test_get_user_or_root_log_file_path(
+        self,
+        m_we_are_currently_root,
+        m_cfg_log_file,
+        m_get_user_log_file,
+        we_are_currently_root,
+        expected,
+    ):
         """
-        Tests that the correct default log_file storage location is retrieved
-        when the user is root.
+        Tests that the correct log_file path is retrieved
+        when the user is root and non-root
         """
-        # test root log file path
-        with mock.patch(
-            "uaclient.util.we_are_currently_root",
-            return_value=True,
-        ):
-            assert (
-                pro_log.get_user_or_root_log_file_path()
-                == "/var/log/ubuntu-advantage.log"
-            )
-        # test default user log file path
-        with mock.patch(
-            "uaclient.util.we_are_currently_root",
-            return_value=False,
-        ):
-            assert (
-                pro_log.get_user_or_root_log_file_path()
-                == "~/.cache/ubuntu-pro/ubuntu-pro.log"
-            )
-        # test custom user log file path
-        with mock.patch(
-            "uaclient.config.UAConfig.log_file", new_callable=mock.PropertyMock
-        ) as mock_log_file:
-            expected_log_file = "/tmp/foo.log"
-            mock_log_file.return_value = expected_log_file
-            with mock.patch(
-                "uaclient.util.we_are_currently_root", return_value=True
-            ) as mock_root:
-                result = pro_log.get_user_or_root_log_file_path()
-                mock_root.assert_called()
-                mock_log_file.assert_called()
-                assert expected_log_file == result
+        m_we_are_currently_root.return_value = we_are_currently_root
+        result = pro_log.get_user_or_root_log_file_path()
+        # ensure mocks are used properly
+        assert m_we_are_currently_root.call_count == 1
+        assert m_cfg_log_file.call_count + m_get_user_log_file.call_count == 1
+        if we_are_currently_root:
+            assert m_cfg_log_file.call_count == 1
+        else:
+            assert m_get_user_log_file.call_count == 1
+        # ensure correct log_file path is returned
+        assert expected == result