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

Side by Side Diff: PRESUBMIT.py

Issue 766713004: Add PRESUBMIT check if modified UMA histogram name can be found (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: comments Created 6 years 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 unified diff | Download patch
« no previous file with comments | « no previous file | PRESUBMIT_test.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Top-level presubmit script for Chromium. 5 """Top-level presubmit script for Chromium.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into gcl. 8 for more details about the presubmit API built into gcl.
9 """ 9 """
10 10
(...skipping 339 matching lines...) Expand 10 before | Expand all | Expand 10 after
350 350
351 for line_num, line in f.ChangedContents(): 351 for line_num, line in f.ChangedContents():
352 if 'UNIT_TEST ' in line or line.endswith('UNIT_TEST'): 352 if 'UNIT_TEST ' in line or line.endswith('UNIT_TEST'):
353 problems.append(' %s:%d' % (f.LocalPath(), line_num)) 353 problems.append(' %s:%d' % (f.LocalPath(), line_num))
354 354
355 if not problems: 355 if not problems:
356 return [] 356 return []
357 return [output_api.PresubmitPromptWarning('UNIT_TEST is only for headers.\n' + 357 return [output_api.PresubmitPromptWarning('UNIT_TEST is only for headers.\n' +
358 '\n'.join(problems))] 358 '\n'.join(problems))]
359 359
360 def _CheckUmaHistogramChanges(input_api, output_api):
361 """Check that UMA histogram names in touched lines can still be found in other
362 lines of the patch or in histograms.xml. Note that this check would not catch
363 the reverse: changes in histograms.xml not matched in the code itself."""
364
365 touched_histograms = []
366 histograms_xml_modifications = []
367 pattern = input_api.re.compile('UMA_HISTOGRAM.*\("(.*)"')
368 for f in input_api.AffectedFiles():
369 # If histograms.xml itself is modified, keep the modified lines for later.
370 if (f.LocalPath().endswith(('histograms.xml'))):
371 histograms_xml_modifications = f.ChangedContents()
372 continue
373 if (not f.LocalPath().endswith(('cc', 'mm', 'cpp'))):
374 continue
375 for line_num, line in f.ChangedContents():
376 found = pattern.search(line)
377 if found:
378 touched_histograms.append([found.group(1), f, line_num])
379
380 # Search for the touched histogram names in the local modifications to
381 # histograms.xml, and if not found on the base file.
382 problems = []
383 for histogram_name, f, line_num in touched_histograms:
384 histogram_name_found = False
385 for line_num, line in histograms_xml_modifications:
386 if histogram_name in line:
387 histogram_name_found = True;
388 break;
389 if histogram_name_found:
390 continue
391
392 with open('tools/metrics/histograms/histograms.xml') as histograms_xml:
393 for line in histograms_xml:
394 if histogram_name in line:
395 histogram_name_found = True;
396 break;
397 if histogram_name_found:
398 continue
399 problems.append(' [%s:%d] %s' % (f.LocalPath(), line_num, histogram_name))
400
401 if not problems:
402 return []
403 return [output_api.PresubmitPromptWarning('Some UMA_HISTOGRAM lines have '
404 'been modified and the associated histogram name has no match in either '
405 'metrics/histograms.xml or the modifications of it:', problems)]
406
360 407
361 def _CheckNoNewWStrings(input_api, output_api): 408 def _CheckNoNewWStrings(input_api, output_api):
362 """Checks to make sure we don't introduce use of wstrings.""" 409 """Checks to make sure we don't introduce use of wstrings."""
363 problems = [] 410 problems = []
364 for f in input_api.AffectedFiles(): 411 for f in input_api.AffectedFiles():
365 if (not f.LocalPath().endswith(('.cc', '.h')) or 412 if (not f.LocalPath().endswith(('.cc', '.h')) or
366 f.LocalPath().endswith(('test.cc', '_win.cc', '_win.h')) or 413 f.LocalPath().endswith(('test.cc', '_win.cc', '_win.h')) or
367 '/win/' in f.LocalPath()): 414 '/win/' in f.LocalPath()):
368 continue 415 continue
369 416
(...skipping 1143 matching lines...) Expand 10 before | Expand all | Expand 10 after
1513 _IPC_ENUM_TRAITS_DEPRECATED, problems)] 1560 _IPC_ENUM_TRAITS_DEPRECATED, problems)]
1514 else: 1561 else:
1515 return [] 1562 return []
1516 1563
1517 1564
1518 def CheckChangeOnUpload(input_api, output_api): 1565 def CheckChangeOnUpload(input_api, output_api):
1519 results = [] 1566 results = []
1520 results.extend(_CommonChecks(input_api, output_api)) 1567 results.extend(_CommonChecks(input_api, output_api))
1521 results.extend(_CheckValidHostsInDEPS(input_api, output_api)) 1568 results.extend(_CheckValidHostsInDEPS(input_api, output_api))
1522 results.extend(_CheckJavaStyle(input_api, output_api)) 1569 results.extend(_CheckJavaStyle(input_api, output_api))
1570 results.extend(_CheckUmaHistogramChanges(input_api, output_api))
1523 return results 1571 return results
1524 1572
1525 1573
1526 def GetTryServerMasterForBot(bot): 1574 def GetTryServerMasterForBot(bot):
1527 """Returns the Try Server master for the given bot. 1575 """Returns the Try Server master for the given bot.
1528 1576
1529 It tries to guess the master from the bot name, but may still fail 1577 It tries to guess the master from the bot name, but may still fail
1530 and return None. There is no longer a default master. 1578 and return None. There is no longer a default master.
1531 """ 1579 """
1532 # Potentially ambiguous bot names are listed explicitly. 1580 # Potentially ambiguous bot names are listed explicitly.
(...skipping 203 matching lines...) Expand 10 before | Expand all | Expand 10 after
1736 ] 1784 ]
1737 1785
1738 # Match things like path/aura/file.cc and path/file_aura.cc. 1786 # Match things like path/aura/file.cc and path/file_aura.cc.
1739 # Same for chromeos. 1787 # Same for chromeos.
1740 if any(re.search(r'[\\\/_](aura|chromeos)', f) for f in files): 1788 if any(re.search(r'[\\\/_](aura|chromeos)', f) for f in files):
1741 builders.extend([ 1789 builders.extend([
1742 'linux_chromeos_asan', 1790 'linux_chromeos_asan',
1743 ]) 1791 ])
1744 1792
1745 return GetDefaultTryConfigs(builders) 1793 return GetDefaultTryConfigs(builders)
OLDNEW
« no previous file with comments | « no previous file | PRESUBMIT_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698