From 861640f6c89b22a2e9b4b4caf341e71f3d5a47b0 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Wed, 31 Oct 2018 19:45:31 +0000 Subject: [PATCH] metrics: Collect git version. Bug: None Change-Id: I73545ad59134c6e5dbeb47fb2e8168a5afc0e497 Reviewed-on: https://chromium-review.googlesource.com/c/1296861 Commit-Queue: Edward Lesmes Reviewed-by: Andrii Shyshkalov --- metrics.py | 9 ++++++++- metrics_utils.py | 16 ++++++++++++++++ tests/metrics_test.py | 21 +++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/metrics.py b/metrics.py index 674c78b9a..bcf033227 100644 --- a/metrics.py +++ b/metrics.py @@ -213,7 +213,14 @@ class MetricsCollector(object): self.add('python_version', metrics_utils.get_python_version()) self.add('host_os', gclient_utils.GetMacWinOrLinux()) self.add('host_arch', detect_host_arch.HostArch()) - self.add('depot_tools_age', metrics_utils.get_repo_timestamp(DEPOT_TOOLS)) + + depot_tools_age = metrics_utils.get_repo_timestamp(DEPOT_TOOLS) + if depot_tools_age is not None: + self.add('depot_tools_age', depot_tools_age) + + git_version = metrics_utils.get_git_version() + if git_version: + self.add('git_version', git_version) self._upload_metrics_data() if exception: diff --git a/metrics_utils.py b/metrics_utils.py index 920934486..2be13bef1 100644 --- a/metrics_utils.py +++ b/metrics_utils.py @@ -138,12 +138,28 @@ KNOWN_HTTP_ARGS = { 'LABELS', } +GIT_VERSION_RE = re.compile( + r'git version (\d)\.(\d{0,2})\.(\d{0,2})' +) + def get_python_version(): """Return the python version in the major.minor.micro format.""" return '{v.major}.{v.minor}.{v.micro}'.format(v=sys.version_info) +def get_git_version(): + """Return the Git version in the major.minor.micro format.""" + p = subprocess2.Popen( + ['git', '--version'], + stdout=subprocess2.PIPE, stderr=subprocess2.PIPE) + stdout, _ = p.communicate() + match = GIT_VERSION_RE.match(stdout) + if not match: + return None + return '%s.%s.%s' % match.groups() + + def return_code_from_exception(exception): """Returns the exit code that would result of raising the exception.""" if exception is None: diff --git a/tests/metrics_test.py b/tests/metrics_test.py index feba7a3c2..ef88b152d 100644 --- a/tests/metrics_test.py +++ b/tests/metrics_test.py @@ -65,9 +65,12 @@ class MetricsCollectorTest(unittest.TestCase): lambda: 'x86').start() mock.patch('metrics_utils.get_repo_timestamp', lambda _: 1234).start() + mock.patch('metrics_utils.get_git_version', + lambda: '2.18.1').start() self.default_metrics = { "python_version": "2.7.13", + "git_version": "2.18.1", "execution_time": 1000, "timestamp": 0, "exit_code": 0, @@ -697,6 +700,24 @@ class MetricsUtilsTest(unittest.TestCase): http_metrics = metrics_utils.extract_http_metrics('', '', 0, 12345.25) self.assertEqual(12345.25, http_metrics['response_time']) + @mock.patch('metrics_utils.subprocess2.Popen') + def test_get_git_version(self, mockPopen): + """Tests that we can get the git version.""" + mockProcess = mock.Mock() + mockProcess.communicate.side_effect = [('git version 2.18.0.123.foo', '')] + mockPopen.side_effect = [mockProcess] + + self.assertEqual('2.18.0', metrics_utils.get_git_version()) + + @mock.patch('metrics_utils.subprocess2.Popen') + def test_get_git_version_unrecognized(self, mockPopen): + """Tests that we can get the git version.""" + mockProcess = mock.Mock() + mockProcess.communicate.side_effect = [('Blah blah blah', 'blah blah')] + mockPopen.side_effect = [mockProcess] + + self.assertIsNone(metrics_utils.get_git_version()) + if __name__ == '__main__': unittest.main()