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

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: comments added 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..8588b2e86985f227f993c499be9e2966706350f0
--- /dev/null
+++ b/chrome/browser/resources/PRESUBMIT.py
@@ -0,0 +1,103 @@
+# Copyright 2014 The Chromium Authors. All rights reserved.
+# 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)
Alexei Svitkine (slow) 2014/11/15 01:07:48 Hmm, maybe name this metric_name rather than actio
gayane -on leave until 09-2017 2014/11/18 18:18:57 Done.
+ is_boolean = IsBoolean(f.NewContents(), action_name)
+ # Search for the matched user action name in |current_actions|.
+ if not IsActionPresent(current_actions, action_name, is_boolean):
+ 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):
+ """Checks if action_name is defined in the actions file.
+
+ This function tries to match the action definition for the provided action
+ name if it is not boolean action. If it is boolean then it tries to match
+ first the disabled action. In case of success it contiunes to search for
+ enabled one.
Alexei Svitkine (slow) 2014/11/15 01:07:48 You can make this shorter by describing the purpos
gayane -on leave until 09-2017 2014/11/18 18:18:57 Done.
+
+ Args:
+ current_actions: The content of the actions.xml file.
+ action_name: The action_name for which the check should be done.
+ is_boolean: The identifer for the action type.
Alexei Svitkine (slow) 2014/11/15 01:07:48 What's an identifier for the action type? It would
gayane -on leave until 09-2017 2014/11/18 18:18:57 Done.
+ """
+ if not is_boolean:
+ action = 'name="{0}"'.format(action_name)
+ return action in current_actions
+
+ 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
Alexei Svitkine (slow) 2014/11/15 01:07:48 Why does "action in current_actions" work for the
gayane -on leave until 09-2017 2014/11/18 18:18:57 Done.
+
+
+def IsBoolean(new_content_lines, action_name):
+ """Check whether action defined in the changed code is boolean or not.
+
+ This function first tries to match the HTML tag in which the action has been
+ mention. Afterwards it tries to match boolean indicators from the remaining
+ text ot the HTML tag.
+
+ Args:
+ new_content_lines: List of changed lines.
+ action_name: Action name for which the check should be done.
+ """
+ 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