diff options
author | mcasas <mcasas@chromium.org> | 2014-12-02 07:37:23 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-12-02 15:37:48 +0000 |
commit | 2ece5270462346b1ac3bccd3bfe5e68d024b98b5 (patch) | |
tree | 84619842df85160e66c4bda5ba9a5b2d6122e165 /PRESUBMIT.py | |
parent | 1e519da432ee8cbedd0926765f8967db2f8c88e4 (diff) | |
download | chromium_src-2ece5270462346b1ac3bccd3bfe5e68d024b98b5.zip chromium_src-2ece5270462346b1ac3bccd3bfe5e68d024b98b5.tar.gz chromium_src-2ece5270462346b1ac3bccd3bfe5e68d024b98b5.tar.bz2 |
Add PRESUBMIT check if modified UMA histogram name can be found
This Presubmit checks if some diffs affect any UMA_HISTOGRAM_*
macro and, if so, checks if the histogram name is to be found
in either tools/metrics/histograms/histograms.xml or in the
CL diffs.
Addresses the problem of someone modifying code and
inadvertently forgetting a corresponding histograms.xml
adaptation, that has happened in the past.
BUG=434420
Review URL: https://codereview.chromium.org/766713004
Cr-Commit-Position: refs/heads/master@{#306388}
Diffstat (limited to 'PRESUBMIT.py')
-rw-r--r-- | PRESUBMIT.py | 48 |
1 files changed, 48 insertions, 0 deletions
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 90d1df4..6762e27 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -357,6 +357,53 @@ def _CheckNoUNIT_TESTInSourceFiles(input_api, output_api): return [output_api.PresubmitPromptWarning('UNIT_TEST is only for headers.\n' + '\n'.join(problems))] +def _CheckUmaHistogramChanges(input_api, output_api): + """Check that UMA histogram names in touched lines can still be found in other + lines of the patch or in histograms.xml. Note that this check would not catch + the reverse: changes in histograms.xml not matched in the code itself.""" + + touched_histograms = [] + histograms_xml_modifications = [] + pattern = input_api.re.compile('UMA_HISTOGRAM.*\("(.*)"') + for f in input_api.AffectedFiles(): + # If histograms.xml itself is modified, keep the modified lines for later. + if (f.LocalPath().endswith(('histograms.xml'))): + histograms_xml_modifications = f.ChangedContents() + continue + if (not f.LocalPath().endswith(('cc', 'mm', 'cpp'))): + continue + for line_num, line in f.ChangedContents(): + found = pattern.search(line) + if found: + touched_histograms.append([found.group(1), f, line_num]) + + # Search for the touched histogram names in the local modifications to + # histograms.xml, and if not found on the base file. + problems = [] + for histogram_name, f, line_num in touched_histograms: + histogram_name_found = False + for line_num, line in histograms_xml_modifications: + if histogram_name in line: + histogram_name_found = True; + break; + if histogram_name_found: + continue + + with open('tools/metrics/histograms/histograms.xml') as histograms_xml: + for line in histograms_xml: + if histogram_name in line: + histogram_name_found = True; + break; + if histogram_name_found: + continue + problems.append(' [%s:%d] %s' % (f.LocalPath(), line_num, histogram_name)) + + if not problems: + return [] + return [output_api.PresubmitPromptWarning('Some UMA_HISTOGRAM lines have ' + 'been modified and the associated histogram name has no match in either ' + 'metrics/histograms.xml or the modifications of it:', problems)] + def _CheckNoNewWStrings(input_api, output_api): """Checks to make sure we don't introduce use of wstrings.""" @@ -1520,6 +1567,7 @@ def CheckChangeOnUpload(input_api, output_api): results.extend(_CommonChecks(input_api, output_api)) results.extend(_CheckValidHostsInDEPS(input_api, output_api)) results.extend(_CheckJavaStyle(input_api, output_api)) + results.extend(_CheckUmaHistogramChanges(input_api, output_api)) return results |