diff options
author | mcasas <mcasas@chromium.org> | 2015-02-04 06:52:19 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-02-04 14:53:08 +0000 |
commit | b7440c2829b8fae8467c110235a7aa8363644ae0 (patch) | |
tree | 24a9789f5069afe566f026d03794c98c6167a0fb /PRESUBMIT.py | |
parent | 869b89123602a51ac523d97054262bc4ab6a6933 (diff) | |
download | chromium_src-b7440c2829b8fae8467c110235a7aa8363644ae0.zip chromium_src-b7440c2829b8fae8467c110235a7aa8363644ae0.tar.gz chromium_src-b7440c2829b8fae8467c110235a7aa8363644ae0.tar.bz2 |
Add PRESUBMIT check if modified UMA histogram name can be found (2)
This CL is an iteration of http://crrev.com/766713004, that got reverted
after http://crbug.com/445265 because it could not cope correctly
with structures of the type:
<histogram_suffixes name="SafeBrowsingStores.SBWhiteLists" separator=".">
<suffix name="CSD" label="CSD"/>
<suffix name="DownloadWhitelist" label="DownloadWhitelist"/>
<affected-histogram name="SB2.DatabaseKilobytes"/>
</histogram_suffixes>
(in particular: affected-histogram-suffixes).
Limitation that should be solved now.
Original description ---------------------------------------------------
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, 445265
Review URL: https://codereview.chromium.org/885783007
Cr-Commit-Position: refs/heads/master@{#314557}
Diffstat (limited to 'PRESUBMIT.py')
-rw-r--r-- | PRESUBMIT.py | 64 |
1 files changed, 64 insertions, 0 deletions
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 3326d8c..8e4880b 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -359,6 +359,69 @@ def _CheckNoUNIT_TESTInSourceFiles(input_api, output_api): '\n'.join(problems))] +def _FindHistogramNameInLine(histogram_name, line): + """Tries to find a histogram name or prefix in a line.""" + if not "affected-histogram" in line: + return histogram_name in line + # A histogram_suffixes tag type has an affected-histogram name as a prefix of + # the histogram_name. + if not '"' in line: + return False + histogram_prefix = line.split('\"')[1] + return histogram_prefix in histogram_name + + +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 histograms.xml file. + unmatched_histograms = [] + for histogram_info in touched_histograms: + histogram_name_found = False + for line_num, line in histograms_xml_modifications: + histogram_name_found = _FindHistogramNameInLine(histogram_info[0], line) + if histogram_name_found: + break + if not histogram_name_found: + unmatched_histograms.append(histogram_info) + + problems = [] + if unmatched_histograms: + with open('tools/metrics/histograms/histograms.xml') as histograms_xml: + for histogram_name, f, line_num in unmatched_histograms: + histogram_name_found = False + for line in histograms_xml: + histogram_name_found = _FindHistogramNameInLine(histogram_name, line) + if histogram_name_found: + break + if not histogram_name_found: + 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.""" problems = [] @@ -1585,6 +1648,7 @@ def CheckChangeOnUpload(input_api, output_api): results.extend(_CheckJavaStyle(input_api, output_api)) results.extend( input_api.canned_checks.CheckGNFormatted(input_api, output_api)) + results.extend(_CheckUmaHistogramChanges(input_api, output_api)) return results |