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

Side by Side Diff: PRESUBMIT.py

Issue 1131903007: [Android log] Promote using hardcoded string tags (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebased, renamed the android group check Created 5 years, 6 months 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 depot_tools. 8 for more details about the presubmit API built into depot_tools.
9 """ 9 """
10 10
(...skipping 1286 matching lines...) Expand 10 before | Expand all | Expand 10 after
1297 import checkstyle 1297 import checkstyle
1298 finally: 1298 finally:
1299 # Restore sys.path to what it was before. 1299 # Restore sys.path to what it was before.
1300 sys.path = original_sys_path 1300 sys.path = original_sys_path
1301 1301
1302 return checkstyle.RunCheckstyle( 1302 return checkstyle.RunCheckstyle(
1303 input_api, output_api, 'tools/android/checkstyle/chromium-style-5.0.xml', 1303 input_api, output_api, 'tools/android/checkstyle/chromium-style-5.0.xml',
1304 black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST) 1304 black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
1305 1305
1306 1306
1307 def _CheckAndroidCrLogUsage(input_api, output_api):
1308 """Checks that new logs using org.chromium.base.Log:
1309 - Are using 'TAG' as variable name for the tags (warn)
1310 - Are using the suggested name format for the tags: "cr.<PackageTag>" (warn)
1311 - Are using a tag that is shorter than 23 characters (error)
1312 """
1313 cr_log_import_pattern = input_api.re.compile(
1314 r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE);
1315 # Extract the tag from lines like `Log.d(TAG, "*");` or `Log.d("TAG", "*");`
1316 cr_log_pattern = input_api.re.compile(r'^\s*Log\.\w\((?P<tag>\"?\w+\"?)\,')
1317 log_decl_pattern = input_api.re.compile(
1318 r'^\s*private static final String TAG = "(?P<name>(.*)")',
1319 input_api.re.MULTILINE)
1320 log_name_pattern = input_api.re.compile(r'^cr[.\w]*')
1321
1322 REF_MSG = ('See base/android/java/src/org/chromium/base/README_logging.md '
1323 'or contact dgn@chromium.org for more info.')
1324 sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',))
1325 tag_errors = []
1326 tag_decl_errors = []
1327 tag_length_errors = []
1328
1329 for f in input_api.AffectedSourceFiles(sources):
1330 file_content = input_api.ReadFile(f)
1331 has_modified_logs = False
1332
1333 # Per line checks
1334 if cr_log_import_pattern.search(file_content):
1335 for line_num, line in f.ChangedContents():
1336
1337 # Check if the new line is doing some logging
1338 match = cr_log_pattern.search(line)
1339 if match:
1340 has_modified_logs = True
1341
1342 # Make sure it uses "TAG"
1343 if not match.group('tag') == 'TAG':
1344 tag_errors.append("%s:%d" % (f.LocalPath(), line_num))
1345
1346 # Per file checks
1347 if has_modified_logs:
1348 # Make sure the tag is using the "cr" prefix and is not too long
1349 match = log_decl_pattern.search(file_content)
1350 tag_name = match.group('name') if match else ''
1351 if not log_name_pattern.search(tag_name ):
1352 tag_decl_errors.append(f.LocalPath())
1353 if len(tag_name) > 23:
1354 tag_length_errors.append(f.LocalPath())
1355
1356 results = []
1357 if tag_decl_errors:
1358 results.append(output_api.PresubmitPromptWarning(
1359 'Please define your tags using the suggested format: .\n'
1360 '"private static final String TAG = "cr.<package tag>".\n' + REF_MSG,
1361 tag_decl_errors))
1362
1363 if tag_length_errors:
1364 results.append(output_api.PresubmitError(
1365 'The tag length is restricted by the system to be at most '
1366 '23 characters.\n' + REF_MSG,
1367 tag_length_errors))
1368
1369 if tag_errors:
1370 results.append(output_api.PresubmitPromptWarning(
1371 'Please use a variable named "TAG" for your log tags.\n' + REF_MSG,
1372 tag_errors))
1373
1374 return results
1375
1376
1377 # TODO(dgn): refactor with _CheckAndroidCrLogUsage
1307 def _CheckNoNewUtilLogUsage(input_api, output_api): 1378 def _CheckNoNewUtilLogUsage(input_api, output_api):
1308 """Checks that new logs are using org.chromium.base.Log.""" 1379 """Checks that new logs are using org.chromium.base.Log."""
1309 1380
1310 chromium_log_import_pattern = input_api.re.compile( 1381 chromium_log_import_pattern = input_api.re.compile(
1311 r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE); 1382 r'^import org\.chromium\.base\.Log;$', input_api.re.MULTILINE);
1312 log_pattern = input_api.re.compile(r'^\s*(android\.util\.)?Log\.\w') 1383 log_pattern = input_api.re.compile(r'^\s*(android\.util\.)?Log\.\w')
1313 sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',)) 1384 sources = lambda x: input_api.FilterSourceFile(x, white_list=(r'.*\.java$',))
1314 1385
1315 errors = [] 1386 errors = []
1316 1387
(...skipping 132 matching lines...) Expand 10 before | Expand all | Expand 10 after
1449 for fpath in input_api.AffectedFiles(file_filter=file_filter): 1520 for fpath in input_api.AffectedFiles(file_filter=file_filter):
1450 for lnum, line in fpath.ChangedContents(): 1521 for lnum, line in fpath.ChangedContents():
1451 for (deprecated, replacement) in _DEPRECATED_JS: 1522 for (deprecated, replacement) in _DEPRECATED_JS:
1452 if deprecated in line: 1523 if deprecated in line:
1453 results.append(output_api.PresubmitError( 1524 results.append(output_api.PresubmitError(
1454 "%s:%d: Use of deprecated JS %s, use %s instead" % 1525 "%s:%d: Use of deprecated JS %s, use %s instead" %
1455 (fpath.LocalPath(), lnum, deprecated, replacement))) 1526 (fpath.LocalPath(), lnum, deprecated, replacement)))
1456 return results 1527 return results
1457 1528
1458 1529
1530 def _AndroidSpecificOnUploadChecks(input_api, output_api):
1531 """Groups checks that target android code."""
1532 results = []
1533 results.extend(_CheckNoNewUtilLogUsage(input_api, output_api))
1534 results.extend(_CheckAndroidCrLogUsage(input_api, output_api))
1535 return results
1536
1537
1459 def _CommonChecks(input_api, output_api): 1538 def _CommonChecks(input_api, output_api):
1460 """Checks common to both upload and commit.""" 1539 """Checks common to both upload and commit."""
1461 results = [] 1540 results = []
1462 results.extend(input_api.canned_checks.PanProjectChecks( 1541 results.extend(input_api.canned_checks.PanProjectChecks(
1463 input_api, output_api, 1542 input_api, output_api,
1464 excluded_paths=_EXCLUDED_PATHS + _TESTRUNNER_PATHS)) 1543 excluded_paths=_EXCLUDED_PATHS + _TESTRUNNER_PATHS))
1465 results.extend(_CheckAuthorizedAuthor(input_api, output_api)) 1544 results.extend(_CheckAuthorizedAuthor(input_api, output_api))
1466 results.extend( 1545 results.extend(
1467 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api)) 1546 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
1468 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api)) 1547 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
(...skipping 252 matching lines...) Expand 10 before | Expand all | Expand 10 after
1721 1800
1722 1801
1723 def CheckChangeOnUpload(input_api, output_api): 1802 def CheckChangeOnUpload(input_api, output_api):
1724 results = [] 1803 results = []
1725 results.extend(_CommonChecks(input_api, output_api)) 1804 results.extend(_CommonChecks(input_api, output_api))
1726 results.extend(_CheckValidHostsInDEPS(input_api, output_api)) 1805 results.extend(_CheckValidHostsInDEPS(input_api, output_api))
1727 results.extend(_CheckJavaStyle(input_api, output_api)) 1806 results.extend(_CheckJavaStyle(input_api, output_api))
1728 results.extend( 1807 results.extend(
1729 input_api.canned_checks.CheckGNFormatted(input_api, output_api)) 1808 input_api.canned_checks.CheckGNFormatted(input_api, output_api))
1730 results.extend(_CheckUmaHistogramChanges(input_api, output_api)) 1809 results.extend(_CheckUmaHistogramChanges(input_api, output_api))
1731 results.extend(_CheckNoNewUtilLogUsage(input_api, output_api)) 1810 results.extend(_AndroidSpecificOnUploadChecks(input_api, output_api))
1732 return results 1811 return results
1733 1812
1734 1813
1735 def GetTryServerMasterForBot(bot): 1814 def GetTryServerMasterForBot(bot):
1736 """Returns the Try Server master for the given bot. 1815 """Returns the Try Server master for the given bot.
1737 1816
1738 It tries to guess the master from the bot name, but may still fail 1817 It tries to guess the master from the bot name, but may still fail
1739 and return None. There is no longer a default master. 1818 and return None. There is no longer a default master.
1740 """ 1819 """
1741 # Potentially ambiguous bot names are listed explicitly. 1820 # Potentially ambiguous bot names are listed explicitly.
(...skipping 66 matching lines...) Expand 10 before | Expand all | Expand 10 after
1808 # (e.g. OWNERS checks before finished code review), and we're 1887 # (e.g. OWNERS checks before finished code review), and we're
1809 # running local presubmit anyway. 1888 # running local presubmit anyway.
1810 if 'presubmit' in builder: 1889 if 'presubmit' in builder:
1811 masters[master].pop(builder) 1890 masters[master].pop(builder)
1812 else: 1891 else:
1813 # Convert testfilter format to the one expected by git-cl-try. 1892 # Convert testfilter format to the one expected by git-cl-try.
1814 testfilter = masters[master][builder].get('testfilter', 'defaulttests') 1893 testfilter = masters[master][builder].get('testfilter', 'defaulttests')
1815 masters[master][builder] = [testfilter] 1894 masters[master][builder] = [testfilter]
1816 1895
1817 return masters 1896 return masters
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