pre-upload: support options for default enabled flags
We pull out hook options when the hook is explicitly enabled, but not
when it's implicitly enabled by default. We haven't had any hooks as
those take options before so no one noticed.
BUG=b:162982485
TEST=unittests pass
Change-Id: Ib94acf043b2b4053ae67e704610a9b0417c8cbf5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/2363878
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index bdacbec..3a759f4 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -2063,20 +2063,26 @@
valid_keys = set(_HOOK_FLAGS.keys())
hooks = _HOOK_FLAGS.copy()
- options = config.options(SECTION) if config.has_section(SECTION) else []
+ hook_overrides = set(
+ config.options(SECTION) if config.has_section(SECTION) else [])
+
+ unknown_keys = hook_overrides - valid_keys
+ if unknown_keys:
+ raise ValueError(f'{_CONFIG_FILE}: [{SECTION}]: unknown keys: '
+ f'{unknown_keys}')
enable_flags = []
disable_flags = []
- for flag in options:
- if flag not in valid_keys:
- raise ValueError('Error: unknown key "%s" in hook section of "%s"' %
- (flag, _CONFIG_FILE))
+ for flag in valid_keys:
+ if flag in hook_overrides:
+ try:
+ enabled = config.getboolean(SECTION, flag)
+ except ValueError as e:
+ raise ValueError('Error: parsing flag "%s" in "%s" failed: %s' %
+ (flag, _CONFIG_FILE, e))
+ else:
+ enabled = True
- try:
- enabled = config.getboolean(SECTION, flag)
- except ValueError as e:
- raise ValueError('Error: parsing flag "%s" in "%s" failed: %s' %
- (flag, _CONFIG_FILE, e))
if enabled:
enable_flags.append(flag)
else:
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 7613973..78d6794 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -8,6 +8,7 @@
from __future__ import print_function
+import configparser
import datetime
import os
import sys
@@ -1862,5 +1863,47 @@
self.assertTrue(called)
+class OverrideHooksProcessing(PreUploadTestCase):
+ """Verify _get_override_hooks processing."""
+
+ @staticmethod
+ def parse(data):
+ """Helper to create a config & parse it."""
+ cfg = configparser.ConfigParser()
+ cfg.read_string(data)
+ return pre_upload._get_override_hooks(cfg)
+
+ def testHooks(self):
+ """Verify we reject unknown hook names (e.g. typos)."""
+ with self.assertRaises(ValueError) as e:
+ self.parse("""
+[Hook Overrides]
+foobar: true
+""")
+ self.assertIn('foobar', str(e.exception))
+
+ def testExplicitDisable(self):
+ """Verify hooks disabled are disabled."""
+ _, disabled = self.parse("""
+[Hook Overrides]
+tab_check: false
+""")
+ self.assertIn(pre_upload._check_no_tabs, disabled)
+
+ def testOptions(self):
+ """Verify hook options are loaded."""
+ enabled, _ = self.parse("""
+[Hook Overrides Options]
+keyword_check: --kw
+""")
+ for func in enabled:
+ if func.__name__ == 'keyword_check':
+ self.assertIn('options', func.keywords)
+ self.assertEqual(func.keywords['options'], ['--kw'])
+ break
+ else:
+ self.fail('could not find "keyword_check" enabled hook')
+
+
if __name__ == '__main__':
cros_test_lib.main(module=__name__)
