Skip to content

Commit

Permalink
Add presubmit check for Google support URL format.
Browse files Browse the repository at this point in the history
See bug for details. tl;dr: we should not use direct links like
support.google.com/chrome/answer/123456

For now this is an ignoreable upload-only prompt.

BUG=679462

Review-Url: https://codereview.chromium.org/2627023003
Cr-Commit-Position: refs/heads/master@{#443255}
  • Loading branch information
estade authored and Commit bot committed Jan 12, 2017
1 parent 88b1b7e commit e17314a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
16 changes: 16 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,21 @@ def _CheckForVersionControlConflicts(input_api, output_api):
'Version control conflict markers found, please resolve.', errors))
return results

def _CheckGoogleSupportAnswerUrl(input_api, output_api):
pattern = input_api.re.compile('support\.google\.com\/chrome.*/answer')
errors = []
for f in input_api.AffectedFiles():
for line_num, line in f.ChangedContents():
if pattern.search(line):
errors.append(' %s:%d %s' % (f.LocalPath(), line_num, line))

results = []
if errors:
results.append(output_api.PresubmitPromptWarning(
'Found Google support URL addressed by answer number. Please replace with '
'a p= identifier instead. See crbug.com/679462\n', errors))
return results


def _CheckHardcodedGoogleHostsInLowerLayers(input_api, output_api):
def FilterFile(affected_file):
Expand Down Expand Up @@ -2298,6 +2313,7 @@ def CheckChangeOnUpload(input_api, output_api):
results.extend(_CheckUmaHistogramChanges(input_api, output_api))
results.extend(_AndroidSpecificOnUploadChecks(input_api, output_api))
results.extend(_CheckSyslogUseWarning(input_api, output_api))
results.extend(_CheckGoogleSupportAnswerUrl(input_api, output_api))
return results


Expand Down
30 changes: 30 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,36 @@ def testCheckAndroidCrLogUsage(self):
self.assertTrue('HasDottedTag.java' in msgs[4].items)
self.assertTrue('HasOldTag.java' in msgs[4].items)

class GoogleAnswerUrlFormatTest(unittest.TestCase):

def testCatchAnswerUrlId(self):
input_api = MockInputApi()
input_api.files = [
MockFile('somewhere/file.cc',
['char* host = '
' "https://support.google.com/chrome/answer/123456";']),
MockFile('somewhere_else/file.cc',
['char* host = '
' "https://support.google.com/chrome/a/answer/123456";']),
]

warnings = PRESUBMIT._CheckGoogleSupportAnswerUrl(
input_api, MockOutputApi())
self.assertEqual(1, len(warnings))
self.assertEqual(2, len(warnings[0].items))

def testAllowAnswerUrlParam(self):
input_api = MockInputApi()
input_api.files = [
MockFile('somewhere/file.cc',
['char* host = '
' "https://support.google.com/chrome/?p=cpn_crash_reports";']),
]

warnings = PRESUBMIT._CheckGoogleSupportAnswerUrl(
input_api, MockOutputApi())
self.assertEqual(0, len(warnings))

class HardcodedGoogleHostsTest(unittest.TestCase):

def testWarnOnAssignedLiterals(self):
Expand Down

0 comments on commit e17314a

Please sign in to comment.