Presubmit check that strings use the correct product name
Motivated by strings in chromium_strings.grd that use "Chrome":
https://crrev.com/c/1294589
Bug: 897868
Change-Id: Ie90f92e67e8801f9e78bbdf478fa727b5fee3e22
Reviewed-on: https://chromium-review.googlesource.com/c/1294419
Reviewed-by: Jochen Eisinger <[email protected]>
Reviewed-by: Zachary Kuznia <[email protected]>
Commit-Queue: Michael Giuffrida <[email protected]>
Cr-Commit-Position: refs/heads/master@{#602910}diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 2391c996..ccfebcf3 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -2991,6 +2991,50 @@
return []
+def _CheckCorrectProductNameInMessages(input_api, output_api):
+ """Check that Chromium-branded strings don't include "Chrome" or vice versa.
+
+ This assumes we won't intentionally reference one product from the other
+ product.
+ """
+ all_problems = []
+ test_cases = [{
+ "filename_postfix": "google_chrome_strings.grd",
+ "correct_name": "Chrome",
+ "incorrect_name": "Chromium",
+ }, {
+ "filename_postfix": "chromium_strings.grd",
+ "correct_name": "Chromium",
+ "incorrect_name": "Chrome",
+ }]
+
+ for test_case in test_cases:
+ problems = []
+ filename_filter = lambda x: x.LocalPath().endswith(
+ test_case["filename_postfix"])
+
+ # Check each new line. Can yield false positives in multiline comments, but
+ # easier than trying to parse the XML because messages can have nested
+ # children, and associating message elements with affected lines is hard.
+ for f in input_api.AffectedSourceFiles(filename_filter):
+ for line_num, line in f.ChangedContents():
+ if "<message" in line or "<!--" in line or "-->" in line:
+ continue
+ if test_case["incorrect_name"] in line:
+ problems.append(
+ "Incorrect product name in %s:%d" % (f.LocalPath(), line_num))
+
+ if problems:
+ message = (
+ "Strings in %s-branded string files should reference \"%s\", not \"%s\""
+ % (test_case["correct_name"], test_case["correct_name"],
+ test_case["incorrect_name"]))
+ all_problems.append(
+ output_api.PresubmitPromptWarning(message, items=problems))
+
+ return all_problems
+
+
def _AndroidSpecificOnUploadChecks(input_api, output_api):
"""Groups checks that target android code."""
results = []
@@ -3068,6 +3112,7 @@
results.extend(input_api.RunTests(
input_api.canned_checks.CheckVPythonSpec(input_api, output_api)))
results.extend(_CheckTranslationScreenshots(input_api, output_api))
+ results.extend(_CheckCorrectProductNameInMessages(input_api, output_api))
for f in input_api.AffectedFiles():
path, name = input_api.os_path.split(f.LocalPath())
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 0fbda0c..0c23bb96d 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -1611,6 +1611,125 @@
self.assertTrue('base/another.h' in warnings[0].items)
+class CorrectProductNameInMessagesTest(unittest.TestCase):
+ def testProductNameInDesc(self):
+ mock_input_api = MockInputApi()
+ mock_input_api.files = [
+ MockAffectedFile('chrome/app/google_chrome_strings.grd', [
+ '<message name="Foo" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ ]),
+ MockAffectedFile('chrome/app/chromium_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chromium!',
+ '</message>',
+ ]),
+ ]
+ warnings = PRESUBMIT._CheckCorrectProductNameInMessages(
+ mock_input_api, MockOutputApi())
+ self.assertEqual(0, len(warnings))
+
+ def testChromeInChromium(self):
+ mock_input_api = MockInputApi()
+ mock_input_api.files = [
+ MockAffectedFile('chrome/app/google_chrome_strings.grd', [
+ '<message name="Foo" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ ]),
+ MockAffectedFile('chrome/app/chromium_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ ]),
+ ]
+ warnings = PRESUBMIT._CheckCorrectProductNameInMessages(
+ mock_input_api, MockOutputApi())
+ self.assertEqual(1, len(warnings))
+ self.assertTrue('chrome/app/chromium_strings.grd' in warnings[0].items[0])
+
+ def testChromiumInChrome(self):
+ mock_input_api = MockInputApi()
+ mock_input_api.files = [
+ MockAffectedFile('chrome/app/google_chrome_strings.grd', [
+ '<message name="Foo" desc="Welcome to Chrome">',
+ ' Welcome to Chromium!',
+ '</message>',
+ ]),
+ MockAffectedFile('chrome/app/chromium_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chromium!',
+ '</message>',
+ ]),
+ ]
+ warnings = PRESUBMIT._CheckCorrectProductNameInMessages(
+ mock_input_api, MockOutputApi())
+ self.assertEqual(1, len(warnings))
+ self.assertTrue(
+ 'chrome/app/google_chrome_strings.grd:2' in warnings[0].items[0])
+
+ def testMultipleInstances(self):
+ mock_input_api = MockInputApi()
+ mock_input_api.files = [
+ MockAffectedFile('chrome/app/chromium_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ '<message name="Baz" desc="A correct message">',
+ ' Chromium is the software you are using.',
+ '</message>',
+ '<message name="Bat" desc="An incorrect message">',
+ ' Google Chrome is the software you are using.',
+ '</message>',
+ ]),
+ ]
+ warnings = PRESUBMIT._CheckCorrectProductNameInMessages(
+ mock_input_api, MockOutputApi())
+ self.assertEqual(1, len(warnings))
+ self.assertTrue(
+ 'chrome/app/chromium_strings.grd:2' in warnings[0].items[0])
+ self.assertTrue(
+ 'chrome/app/chromium_strings.grd:8' in warnings[0].items[1])
+
+ def testMultipleWarnings(self):
+ mock_input_api = MockInputApi()
+ mock_input_api.files = [
+ MockAffectedFile('chrome/app/chromium_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ '<message name="Baz" desc="A correct message">',
+ ' Chromium is the software you are using.',
+ '</message>',
+ '<message name="Bat" desc="An incorrect message">',
+ ' Google Chrome is the software you are using.',
+ '</message>',
+ ]),
+ MockAffectedFile('components/components_google_chrome_strings.grd', [
+ '<message name="Bar" desc="Welcome to Chrome">',
+ ' Welcome to Chrome!',
+ '</message>',
+ '<message name="Baz" desc="A correct message">',
+ ' Chromium is the software you are using.',
+ '</message>',
+ '<message name="Bat" desc="An incorrect message">',
+ ' Google Chrome is the software you are using.',
+ '</message>',
+ ]),
+ ]
+ warnings = PRESUBMIT._CheckCorrectProductNameInMessages(
+ mock_input_api, MockOutputApi())
+ self.assertEqual(2, len(warnings))
+ self.assertTrue(
+ 'components/components_google_chrome_strings.grd:5'
+ in warnings[0].items[0])
+ self.assertTrue(
+ 'chrome/app/chromium_strings.grd:2' in warnings[1].items[0])
+ self.assertTrue(
+ 'chrome/app/chromium_strings.grd:8' in warnings[1].items[1])
+
+
class MojoManifestOwnerTest(unittest.TestCase):
def testMojoManifestChangeNeedsSecurityOwner(self):
mock_input_api = MockInputApi()
