SERVER-71506 Refactor tooling metrics

4 files changed, 136 insertions, 163 deletions

diff --git a/buildscripts/tests/tooling_metrics/test_metrics_datatypes.py b/buildscripts/tests/tooling_metrics/test_metrics_datatypes.py
index b584b77e647..3e181eb4971 100644
--- a/buildscripts/tests/tooling_metrics/test_metrics_datatypes.py
+++ b/buildscripts/tests/tooling_metrics/test_metrics_datatypes.py
@@ -1,5 +1,7 @@
 """Unit tests for metrics_datatypes.py."""
 from datetime import datetime
+import os
+import sys
 import unittest
 from unittest.mock import patch
 
@@ -9,6 +11,12 @@ import buildscripts.metrics.metrics_datatypes as under_test
 
 # pylint: disable=unused-argument
 
+# Metrics collection is not supported for Windows
+if os.name == "nt":
+    sys.exit()
+
+MOCK_EXIT_HOOK = MagicMock(exit_code=0)
+
 
 @patch("buildscripts.metrics.metrics_datatypes.BuildInfo._get_scons_artifact_dir",
        return_value='/test')
@@ -18,94 +26,89 @@ class TestBuildInfo(unittest.TestCase):
     @patch("buildscripts.metrics.metrics_datatypes.BuildInfo._get_scons_options_dict",
            return_value={'opt': 'opt'})
     def test_build_info_valid(self, mock_env, mock_options, mock_artifact_dir):
-        build_info = under_test.BuildInfo.get_scons_build_info(datetime.utcnow(), MagicMock(),
-                                                               MagicMock(), MagicMock(),
-                                                               MagicMock())
+        build_info = under_test.BuildInfo.generate_metrics(datetime.utcnow(), MagicMock(),
+                                                           MagicMock(), MagicMock(), MagicMock())
         assert not build_info.is_malformed()
 
     def test_build_info_malformed(self, mock_artifact_dir):
-        build_info = under_test.BuildInfo.get_scons_build_info(datetime.utcnow(), MagicMock(),
-                                                               MagicMock(), MagicMock(),
-                                                               MagicMock())
+        build_info = under_test.BuildInfo.generate_metrics(datetime.utcnow(), MagicMock(),
+                                                           MagicMock(), MagicMock(), MagicMock())
         assert build_info.is_malformed()
 
 
-class TestExitInfo(unittest.TestCase):
-    @patch("sys.exc_info", return_value=(None, None, None))
-    def test_resmoke_no_exc_info(self, mock_exc_info):
-        exit_info = under_test.ExitInfo.get_resmoke_exit_info()
-        assert not exit_info.is_malformed()
-
-    @patch("sys.exc_info", return_value=(None, ValueError(), None))
-    def test_resmoke_with_exc_info(self, mock_exc_info):
-        exit_info = under_test.ExitInfo.get_resmoke_exit_info()
-        assert not exit_info.is_malformed()
-
-    def test_scons_exit_info_valid(self):
-        exit_info = under_test.ExitInfo.get_scons_exit_info(0)
-        assert not exit_info.is_malformed()
-
-    def test_scons_exit_info_malformed(self):
-        exit_info = under_test.ExitInfo.get_scons_exit_info('string')
-        assert exit_info.is_malformed()
-
-
 class TestHostInfo(unittest.TestCase):
     @patch("buildscripts.metrics.metrics_datatypes.HostInfo._get_memory", side_effect=Exception())
     def test_host_info_with_exc(self, mock_get_memory):
-        host_info = under_test.HostInfo.get_host_info()
+        host_info = under_test.HostInfo.generate_metrics()
         assert host_info.is_malformed()
 
     # Mock this so that it passes when running the 'buildscripts_test' suite on Windows
     @patch("buildscripts.metrics.metrics_datatypes.HostInfo._get_memory", return_value=30)
     def test_host_info_no_exc(self, mock_get_memory):
-        host_info = under_test.HostInfo.get_host_info()
+        host_info = under_test.HostInfo.generate_metrics()
         assert not host_info.is_malformed()
 
 
 class TestGitInfo(unittest.TestCase):
     @patch("git.Repo", side_effect=Exception())
     def test_git_info_with_exc(self, mock_repo):
-        git_info = under_test.GitInfo.get_git_info('.')
+        git_info = under_test.GitInfo.generate_metrics('.')
         assert git_info.is_malformed()
 
     def test_git_info_no_exc(self):
-        git_info = under_test.GitInfo.get_git_info('.')
+        git_info = under_test.GitInfo.generate_metrics('.')
         assert not git_info.is_malformed()
 
     @patch("git.refs.symbolic.SymbolicReference.is_detached", True)
     def test_git_info_detached_head(self):
-        git_info = under_test.GitInfo.get_git_info('.')
+        git_info = under_test.GitInfo.generate_metrics('.')
         assert not git_info.is_malformed()
 
 
-# Mock this so that it passes when running the 'buildscripts_test' suite on Windows
-@patch("buildscripts.metrics.metrics_datatypes.HostInfo._get_memory", return_value=30)
-class TestToolingMetrics(unittest.TestCase):
+class TestResmokeToolingMetrics(unittest.TestCase):
     @patch("socket.gethostname", side_effect=Exception())
-    def test_resmoke_tooling_metrics_with_exc(self, mock_gethostname, mock_get_memory):
-        tooling_metrics = under_test.ToolingMetrics.get_resmoke_metrics(datetime.utcnow())
+    def test_resmoke_tooling_metrics_valid(self, mock_gethostname):
+        tooling_metrics = under_test.ResmokeToolingMetrics.generate_metrics(
+            datetime.utcnow(),
+            MOCK_EXIT_HOOK,
+        )
         assert tooling_metrics.is_malformed()
 
-    def test_resmoke_tooling_metrics_no_exc(self, mock_get_memory):
-        tooling_metrics = under_test.ToolingMetrics.get_resmoke_metrics(datetime.utcnow())
+    def test_resmoke_tooling_metrics_malformed(self):
+        tooling_metrics = under_test.ResmokeToolingMetrics.generate_metrics(
+            datetime.utcnow(),
+            MOCK_EXIT_HOOK,
+        )
         assert not tooling_metrics.is_malformed()
 
+
+class TestSConsToolingMetrics(unittest.TestCase):
     @patch("buildscripts.metrics.metrics_datatypes.BuildInfo._get_scons_artifact_dir",
            return_value='/test')
     @patch("buildscripts.metrics.metrics_datatypes.BuildInfo._get_scons_env_vars_dict",
            return_value={'env': 'env'})
     @patch("buildscripts.metrics.metrics_datatypes.BuildInfo._get_scons_options_dict",
            return_value={'opt': 'opt'})
-    def test_scons_tooling_metrics_valid(self, mock_options, mock_env, mock_artifact_dir,
-                                         mock_get_memory):
+    def test_scons_tooling_metrics_valid(self, mock_options, mock_env, mock_artifact_dir):
         parser = MagicMock()
         parser.parse_args = MagicMock(return_value={"opt1": "val1"})
-        tooling_metrics = under_test.ToolingMetrics.get_scons_metrics(
-            datetime.utcnow(), {'env': 'env'}, {'opts': 'opts'}, parser, ['test1', 'test2'], 0)
+        tooling_metrics = under_test.SConsToolingMetrics.generate_metrics(
+            datetime.utcnow(),
+            {'env': 'env'},
+            {'opts': 'opts'},
+            parser,
+            ['test1', 'test2'],
+            MOCK_EXIT_HOOK,
+        )
         assert not tooling_metrics.is_malformed()
 
-    def test_scons_tooling_metrics_malformed(self, mock_get_memory):
-        tooling_metrics = under_test.ToolingMetrics.get_scons_metrics(
-            datetime.utcnow(), {'env': 'env'}, {'opts': 'opts'}, None, [], 0)
+    def test_scons_tooling_metrics_malformed(self):
+        tooling_metrics = under_test.SConsToolingMetrics.generate_metrics(
+            datetime.utcnow(),
+            {'env': 'env'},
+            {'opts': 'opts'},
+            None,
+            [],
+            MOCK_EXIT_HOOK,
+        )
         assert tooling_metrics.is_malformed()
diff --git a/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py b/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
index cc3b909381f..c3d7468e90a 100644
--- a/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
+++ b/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
@@ -3,11 +3,8 @@ import os
 import sys
 import unittest
 from unittest.mock import patch
-import mongomock
-import pymongo
 
-import buildscripts.metrics.resmoke_tooling_metrics as under_test
-from buildscripts.resmoke import entrypoint as resmoke_entrypoint
+import buildscripts.resmoke as under_test
 
 TEST_INTERNAL_TOOLING_METRICS_HOSTNAME = 'mongodb://testing:27017'
 CURRENT_DATE_TIME = datetime(2022, 10, 4)
@@ -19,37 +16,32 @@ if os.name == "nt":
     sys.exit()
 
 
-@patch("buildscripts.metrics.tooling_metrics_utils.INTERNAL_TOOLING_METRICS_HOSTNAME",
-       TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
 @patch("buildscripts.resmokelib.logging.flush._FLUSH_THREAD", None)
-class TestResmokeMetricsCollection(unittest.TestCase):
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics", return_value=True)
+@patch("atexit.register")
+class TestResmokeAtExitMetricsCollection(unittest.TestCase):
+    @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=True)
+    def test_resmoke_at_exit_metrics_collection(self, mock_should_collect_metrics,
+                                                mock_atexit_register):
+        under_test.entrypoint()
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
+        assert "_save_metrics" in atexit_functions
+
+    @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=False)
+    def test_no_resmoke_at_exit_metrics_collection(self, mock_should_collect_metrics,
+                                                   mock_atexit_register):
+        under_test.entrypoint()
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
+        assert "_save_metrics" not in atexit_functions
+
     @patch("sys.argv", ['buildscripts/resmoke.py', 'run', '--suite', 'buildscripts_test'])
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=True)
     @patch("buildscripts.resmokelib.testing.executor.TestSuiteExecutor._run_tests",
            side_effect=Exception())
-    def test_resmoke_metrics_collection_exc(self, mock_executor_run, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        with self.assertRaises(SystemExit):
-            resmoke_entrypoint()
-        assert client.metrics.tooling_metrics.find_one()
-
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics", return_value=True)
-    @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
-    def test_resmoke_metrics_collection(self, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        resmoke_entrypoint()
-        assert client.metrics.tooling_metrics.find_one()
-
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics",
-           return_value=False)
-    @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
-    def test_no_resmoke_metrics_collection(self, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        resmoke_entrypoint()
-        assert not client.metrics.tooling_metrics.find_one()
+    def test_resmoke_at_exit_metrics_collection_exc(
+            self, mock_exc_method, mock_should_collect_metrics, mock_atexit_register):
+        with self.assertRaises(SystemExit) as _:
+            under_test.entrypoint()
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
+        assert "_save_metrics" in atexit_functions
diff --git a/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py b/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
index c50c89f85ba..ab23c951f2e 100644
--- a/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
+++ b/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
@@ -1,15 +1,8 @@
-from datetime import datetime
 import os
 import sys
 import unittest
-from unittest.mock import MagicMock, patch
-import mongomock
-import pymongo
-import buildscripts.metrics.scons_tooling_metrics as under_test
-from buildscripts.scons import entrypoint as scons_entrypoint
-
-TEST_INTERNAL_TOOLING_METRICS_HOSTNAME = 'mongodb://testing:27017'
-CURRENT_DATE_TIME = datetime(2022, 10, 4)
+from unittest.mock import patch
+import buildscripts.scons as under_test
 
 # pylint: disable=unused-argument
 # pylint: disable=protected-access
@@ -23,52 +16,29 @@ if os.name == "nt":
     'buildscripts/scons.py', "CC=/opt/mongodbtoolchain/v3/bin/gcc",
     "CXX=/opt/mongodbtoolchain/v3/bin/g++", "NINJA_PREFIX=test_success", "--ninja"
 ])
-@patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
 @patch("atexit.register")
 class TestSconsAtExitMetricsCollection(unittest.TestCase):
-    def test_scons_at_exit_metrics_collection(self, mock_atexit_register,
-                                              mock_should_collect_metrics):
-        with self.assertRaises(SystemExit) as context:
-            scons_entrypoint()
-        assert context.exception.code == 0
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=True)
+    def test_scons_at_exit_metrics_collection(self, mock_should_collect_metrics,
+                                              mock_atexit_register):
+        with self.assertRaises(SystemExit) as _:
+            under_test.entrypoint()
         atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
-        assert "_save_scons_tooling_metrics" in atexit_functions
+        assert "_save_metrics" in atexit_functions
 
-    @patch("buildscripts.moduleconfig.get_module_sconscripts", side_effect=Exception())
-    def test_scons_at_exit_metrics_collection_exc(self, mock_method, mock_atexit_register,
-                                                  mock_should_collect_metrics):
-        with self.assertRaises(SystemExit) as context:
-            scons_entrypoint()
-        assert context.exception.code == 2
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=False)
+    def test_no_scons_at_exit_metrics_collection(self, mock_should_collect_metrics,
+                                                 mock_atexit_register):
+        with self.assertRaises(SystemExit) as _:
+            under_test.entrypoint()
         atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
-        assert "_save_scons_tooling_metrics" in atexit_functions
-
-
-@patch("buildscripts.metrics.tooling_metrics_utils.INTERNAL_TOOLING_METRICS_HOSTNAME",
-       TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-class TestSconsMetricsCollection(unittest.TestCase):
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
-    def test_scons_metrics_collection_success(self, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        under_test._save_scons_tooling_metrics(CURRENT_DATE_TIME, None, None, None, None,
-                                               MagicMock(exit_code=0))
-        assert client.metrics.tooling_metrics.find_one()
+        assert "_save_metrics" not in atexit_functions
 
-    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    def test_scons_metrics_collection_fail(self, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        under_test._save_scons_tooling_metrics(None, None, None, None, None, None)
-        assert not client.metrics.tooling_metrics.find_one()
-
-    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=False)
-    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    def test_no_scons_metrics_collection(self, mock_should_collect_metrics):
-        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-        assert not client.metrics.tooling_metrics.find_one()
-        under_test._save_scons_tooling_metrics(CURRENT_DATE_TIME, None, None, None, None,
-                                               MagicMock(exit_code=0))
-        assert not client.metrics.tooling_metrics.find_one()
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=True)
+    @patch("buildscripts.moduleconfig.get_module_sconscripts", side_effect=Exception())
+    def test_scons_at_exit_metrics_collection_exc(
+            self, mock_exc_method, mock_should_collect_metrics, mock_atexit_register):
+        with self.assertRaises(SystemExit) as _:
+            under_test.entrypoint()
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit_register.call_args_list]
+        assert "_save_metrics" in atexit_functions
diff --git a/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py b/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
index b540a529d78..c705059194b 100644
--- a/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
+++ b/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
@@ -1,67 +1,75 @@
 """Unit tests for tooling_metrics.py."""
-import asyncio
 from datetime import datetime
 import os
 import sys
 import unittest
-from unittest.mock import patch
+from unittest.mock import mock_open, patch
+from mock import MagicMock
 import mongomock
 import pymongo
-from buildscripts.metrics.metrics_datatypes import ToolingMetrics
+from buildscripts.metrics.metrics_datatypes import ResmokeToolingMetrics, SConsToolingMetrics
 import buildscripts.metrics.tooling_metrics_utils as under_test
 
 # pylint: disable=unused-argument
 # pylint: disable=protected-access
 
 TEST_INTERNAL_TOOLING_METRICS_HOSTNAME = 'mongodb://testing:27017'
-CURRENT_DATE_TIME = datetime(2022, 10, 4)
+RESMOKE_METRICS_ARGS = {
+    "utc_starttime": datetime(2022, 10, 4),
+    "exit_hook": MagicMock(exit_code=0),
+}
 
 # Metrics collection is not supported for Windows
 if os.name == "nt":
     sys.exit()
 
 
+@patch("atexit.register")
+class TestRegisterMetricsCollectionAtExit(unittest.TestCase):
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=True)
+    def test_register_metrics_collection(self, mock_should_collect_metrics, mock_atexit):
+        under_test.register_metrics_collection_atexit(ResmokeToolingMetrics.generate_metrics,
+                                                      RESMOKE_METRICS_ARGS)
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit.call_args_list]
+        assert "_save_metrics" in atexit_functions
+
+    @patch("buildscripts.metrics.tooling_metrics_utils._should_collect_metrics", return_value=False)
+    def test_no_register_metrics_collection(self, mock_should_collect_metrics, mock_atexit):
+        under_test.register_metrics_collection_atexit(ResmokeToolingMetrics.generate_metrics,
+                                                      RESMOKE_METRICS_ARGS)
+        atexit_functions = [call[0][0].__name__ for call in mock_atexit.call_args_list]
+        assert "_save_metrics" not in atexit_functions
+
+
 @patch("buildscripts.metrics.tooling_metrics_utils.INTERNAL_TOOLING_METRICS_HOSTNAME",
        TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
 class TestSaveToolingMetrics(unittest.TestCase):
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    def test_on_virtual_workstation(self):
-        under_test.save_tooling_metrics(ToolingMetrics.get_resmoke_metrics(CURRENT_DATE_TIME))
+    def test_save_resmoke_metrics(self):
+        under_test._save_metrics(ResmokeToolingMetrics.generate_metrics, RESMOKE_METRICS_ARGS)
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert client.metrics.tooling_metrics.find_one()
 
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    @patch("buildscripts.metrics.tooling_metrics_utils._save_metrics",
-           side_effect=pymongo.errors.WriteError(error="Error Information"))
-    def test_exception_caught(self, mock_save_metrics):
-        with self.assertLogs('tooling_metrics_utils') as cm:
-            under_test.save_tooling_metrics(ToolingMetrics.get_resmoke_metrics(CURRENT_DATE_TIME))
+    @patch("buildscripts.metrics.tooling_metrics_utils._get_internal_tooling_metrics_client",
+           side_effect=pymongo.errors.ServerSelectionTimeoutError(message="Error Information"))
+    def test_save_metrics_with_exc(self, mock_save_metrics):
+        with self.assertLogs('tooling_metrics') as cm:
+            under_test._save_metrics(ResmokeToolingMetrics.generate_metrics, RESMOKE_METRICS_ARGS)
         assert "Error Information" in cm.output[0]
-        assert "Unexpected: Tooling metrics collection is not available" in cm.output[0]
+        assert "Internal Metrics Collection Failed" in cm.output[0]
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert not client.metrics.tooling_metrics.find_one()
 
 
 class TestIsVirtualWorkstation(unittest.TestCase):
-    @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=False)
-    @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=True)
-    def test_no_toolchain_has_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test._is_virtual_workstation()
-
-    @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=True)
-    @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=True)
-    def test_has_toolchain_has_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert under_test._is_virtual_workstation()
-
-    @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=True)
-    @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=False)
-    def test_has_toolchain_no_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test._is_virtual_workstation()
+    @patch("builtins.open", mock_open(read_data="ubuntu1804-workstation"))
+    def test_is_virtual_workstation(self):
+        assert under_test._is_virtual_workstation() is True
 
-    @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=False)
-    @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=False)
-    def test_no_toolchain_no_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test._is_virtual_workstation()
+    @patch("builtins.open", mock_open(read_data="test"))
+    def test_is_not_virtual_workstation(self):
+        assert under_test._is_virtual_workstation() is False
 
 
 class TestHasMetricsOptOut(unittest.TestCase):
@@ -78,9 +86,9 @@ class TestShouldCollectMetrics(unittest.TestCase):
     @patch("buildscripts.metrics.tooling_metrics_utils._is_virtual_workstation", return_value=True)
     @patch("buildscripts.metrics.tooling_metrics_utils._has_metrics_opt_out", return_value=False)
     def test_should_collect_metrics(self, mock_opt_out, mock_is_virtual_env):
-        assert under_test.should_collect_metrics()
+        assert under_test._should_collect_metrics()
 
     @patch("buildscripts.metrics.tooling_metrics_utils._is_virtual_workstation", return_value=True)
     @patch("buildscripts.metrics.tooling_metrics_utils._has_metrics_opt_out", return_value=True)
     def test_no_collect_metrics_opt_out(self, mock_opt_out, mock_is_virtual_env):
-        assert not under_test.should_collect_metrics()
+        assert not under_test._should_collect_metrics()