Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2172)

Unified Diff: chrome/browser/resources/PRESUBMIT.py

Issue 719463003: Presubmit checks for user actions intorduced in HTML files. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: refactoring, better action search Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/test/data/presubmit/actions.xml ('k') | chrome/browser/resources/PRESUBMIT_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/resources/PRESUBMIT.py
diff --git a/chrome/browser/resources/PRESUBMIT.py b/chrome/browser/resources/PRESUBMIT.py
new file mode 100644
index 0000000000000000000000000000000000000000..928a0f3dc89a8c3a941a79e21d2607c6a516a5e0
--- /dev/null
+++ b/chrome/browser/resources/PRESUBMIT.py
@@ -0,0 +1,80 @@
+# Copyright (c) 2014 The Chromium Authors. All rights reserved.
Alexei Svitkine (slow) 2014/11/15 00:15:47 Nit: In new files, we don't include the (c) - plea
gayane -on leave until 09-2017 2014/11/15 00:50:33 Done.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""Presubmit script for HTML files in chrome/browser/resources.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into gcl.
+"""
+
+import re
+
+ACTION_XML_PATH = '../../../tools/metrics/actions/actions.xml'
+
+def CheckUserActionUpdate(input_api, output_api, action_xml_path):
+ """Checks if any new user action has been added."""
+ if any('actions.xml' == input_api.os_path.basename(f.LocalPath()) for f in
+ input_api.AffectedFiles()):
+ # If actions.xml is already included in the changelist, the PRESUBMIT
+ # for actions.xml will do a more complete presubmit check.
+ return []
+
+ file_filter = lambda f: f.LocalPath().endswith(('.html'))
+ action_re = r'(^|\s+)metric\s*=\s*"([^ ]*)"'
+ current_actions = None
+ for f in input_api.AffectedFiles(file_filter=file_filter):
+ for line_num, line in f.ChangedContents():
+ match = input_api.re.search(action_re, line)
+ if match:
+ # Loads contents in tools/metrics/actions/actions.xml to memory. It's
+ # loaded only once.
+ if not current_actions:
+ with open(action_xml_path) as actions_f:
+ current_actions = actions_f.read()
+ action_name = match.group(2)
+
+ # Search for the matched user action name in |current_actions|.
+ if not IsActionPresent(current_actions, action_name,
+ IsBoolean(f.NewContents(), action_name)):
Alexei Svitkine (slow) 2014/11/15 00:15:47 Nit: If you make is_boolean a local var above, doe
gayane -on leave until 09-2017 2014/11/15 00:50:33 Done.
+ return [output_api.PresubmitPromptWarning(
+ 'File %s line %d: %s is missing in '
+ 'tools/metrics/actions/actions.xml. Please run '
+ 'tools/metrics/actions/extract_actions.py to update.'
+ % (f.LocalPath(), line_num, action_name), [])]
+ return []
+
+
+def IsActionPresent(current_actions, action_name, is_boolean):
Alexei Svitkine (slow) 2014/11/15 00:15:47 Please add some short comments above these methods
gayane -on leave until 09-2017 2014/11/15 00:50:33 Done.
+ if not is_boolean:
+ action = 'name="{0}"'.format(action_name)
+ return action in current_actions
+ else:
Alexei Svitkine (slow) 2014/11/15 00:15:47 Nit: if there's an early return, remove the else a
gayane -on leave until 09-2017 2014/11/15 00:50:33 Done.
+ action_disabled = 'name="{0}_Disabled"'.format(action_name)
+ action_enabled = 'name="{0}_Enabled"'.format(action_name)
+
+ pos = current_actions.find(action_disabled)
+ return pos >= 0 and current_actions.find(action_enabled, pos) >= 0
+
+
+def IsBoolean(new_content_lines, action_name):
+ new_content = '\n'.join(new_content_lines)
+
+ html_element_re = r'<(.*?)(^|\s+)metric\s*=\s*"%s"(.*?)>' % (action_name)
+ type_re = r'datatype="boolean"|type="checkbox"|type="radio".*?'\
+ 'value=("true"|"false")'
+
+ match = re.search(html_element_re, new_content)
+ if match:
+ left_content = match.group(1)
+ right_content = match.group(3)
+ if re.search(type_re, left_content) or re.search(type_re, right_content):
+ return True
+ return False
+
+
+def CheckChangeOnUpload(input_api, output_api):
+ return CheckUserActionUpdate(input_api, output_api, ACTION_XML_PATH)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+ return CheckUserActionUpdate(input_api, output_api, ACTION_XML_PATH)
« no previous file with comments | « base/test/data/presubmit/actions.xml ('k') | chrome/browser/resources/PRESUBMIT_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698