pre-upload: add COMMIT-QUEUE.ini parse check
BUG=chromium:655381
TEST=None
Change-Id: I17aa70eeb519dde79e7c17266e86e3ed0a0f87f9
Reviewed-on: https://chromium-review.googlesource.com/435579
Commit-Ready: Aviv Keshet <akeshet@chromium.org>
Tested-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index be40f96..734b626 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -20,6 +20,7 @@
import re
import sys
import stat
+import StringIO
from errors import (VerifyException, HookFailure, PrintErrorForProject,
PrintErrorsForCommit)
@@ -31,6 +32,7 @@
sys.path.insert(0, os.path.join(os.path.dirname(sys.argv[0]), '..', '..'))
from chromite.lib import commandline
+from chromite.lib import constants
from chromite.lib import cros_build_lib
from chromite.lib import git
from chromite.lib import osutils
@@ -1229,6 +1231,28 @@
return HookFailure(msg)
+def _check_cq_ini_well_formed(_project, commit):
+ """Check that any modified COMMIT-QUEUE.ini files are well formed."""
+ pattern = '.*' + constants.CQ_CONFIG_FILENAME
+ files = _filter_files(_get_affected_files(commit), (pattern,))
+
+ # TODO(akeshet): Check not only that the file is parseable, but that all the
+ # pre-cq configs it requests are existing ones.
+ for f in files:
+ try:
+ parser = ConfigParser.SafeConfigParser()
+ # Prior to python3, ConfigParser has no read_string method, so we must
+ # pass it either a file path or file like object. And we must use
+ # _get_file_content to fetch file contents to ensure we are examining the
+ # commit diff, rather than whatever's on disk.
+ contents = _get_file_content(f, commit)
+ parser.readfp(StringIO.StringIO(contents))
+ except ConfigParser.Error as e:
+ msg = ('Unable to parse COMMIT-QUEUE.ini file at %s due to %s.' %
+ (f, e))
+ return HookFailure(msg)
+
+
def _run_project_hook_script(script, project, commit):
"""Runs a project hook script.
@@ -1313,6 +1337,8 @@
# A list of hooks that are not project-specific
_COMMON_HOOKS = [
+ _check_cq_ini_well_formed,
+ _check_cros_license,
_check_ebuild_eapi,
_check_ebuild_keywords,
_check_ebuild_licenses,
@@ -1320,12 +1346,11 @@
_check_for_uprev,
_check_gofmt,
_check_layout_conf,
- _check_cros_license,
_check_no_long_lines,
_check_no_stray_whitespace,
_check_no_tabs,
- _check_tabbed_indents,
_check_portage_make_use_var,
+ _check_tabbed_indents,
]
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index c8efbc1..44b47fd 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -202,6 +202,46 @@
self.assertFalse(failure)
+class CheckConfigParsing(cros_test_lib.MockTestCase):
+ """Tests _check_cq_ini_well_formed."""
+
+ def setUp(self):
+ self.file_mock = self.PatchObject(pre_upload, '_get_affected_files')
+ self.content_mock = self.PatchObject(pre_upload, '_get_file_content')
+
+ def testIgnoreIrrelevantFile(self):
+ self.file_mock.return_value = ['unrelated_file.ini']
+ self.content_mock.return_value = '^$malformed gibberish^^&'
+ self.assertEqual(pre_upload._check_cq_ini_well_formed('PROJECT', 'COMMIT'),
+ None)
+
+ def testWellformedFile(self):
+ self.file_mock.return_value = ['COMMIT-QUEUE.ini']
+ self.content_mock.return_value = """#
+# Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# Per-project Commit Queue settings.
+# Documentation: http://goo.gl/4rZhAx
+
+[GENERAL]
+
+# Stages to ignore in the commit queue. If these steps break, your CL will be
+# submitted anyway. Use with caution.
+# The files in here currently only get tested via internal canaries.
+ignored-stages: UNitTest HWTest VMTest UploadPrebuilts Archive"""
+
+ self.assertEqual(pre_upload._check_cq_ini_well_formed('PROJECT', 'COMMIT'),
+ None)
+
+ def testMalformedFile(self):
+ self.file_mock.return_value = ['COMMIT-QUEUE.ini']
+ self.content_mock.return_value = '^$malformed gibberish^^&'
+ m = pre_upload._check_cq_ini_well_formed('PROJECT', 'COMMIT')
+ self.assertTrue(isinstance(m, pre_upload.HookFailure))
+
+
class CheckPortageMakeUseVar(cros_test_lib.MockTestCase):
"""Tests for _check_portage_make_use_var."""
