PRESUBMIT: Add crosapi need browser test check
When a new crosapi mojom file being added, we expect there should also be a browser test being added. This is a non-blocker check and only happens during git cl upload. See my local test: :~/chromium/src$ git diff HEAD~1 diff --git a/chromeos/crosapi/mojom/newfile.mojom b/chromeos/crosapi/mojom/newfile.mojom new file mode 100644 index 0000000000000..fa1237e46e92b --- /dev/null +++ b/chromeos/crosapi/mojom/newfile.mojom @@ -0,0 +1,4 @@ +// Copyright 2022 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// :~/chromium/src$ git cl upload Running Python 3 presubmit upload checks ... ** Presubmit Warnings: 1 ** You are adding a new crosapi, but there is no file ends with browsertest.cc file being added. It is important to add crosapi browser test coverage to avoid version skew issues. Check //docs/lacros/test_instructions.md for more information. Presubmit checks took 2.5s to calculate. There were Python 3 presubmit warnings. Are you sure you wish to continue? (y/N): Bug: 1402823 Change-Id: Ibcac5f90fb7332cf7199e04a8eab48fda3ac5eb4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4118248 Reviewed-by: Erik Chen <erikchen@chromium.org> Reviewed-by: Ben Pastene <bpastene@chromium.org> Commit-Queue: Sven Zheng <svenzheng@chromium.org> Reviewed-by: Bruce Dawson <brucedawson@chromium.org> Cr-Commit-Position: refs/heads/main@{#1086084}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
42b1ba0a0a
commit
76a79ea364
28
PRESUBMIT.py
28
PRESUBMIT.py
@ -1657,6 +1657,10 @@ def _IsXmlOrGrdFile(input_api, file_path):
|
||||
return ext in ('.grd', '.xml')
|
||||
|
||||
|
||||
def _IsMojomFile(input_api, file_path):
|
||||
return input_api.os_path.splitext(file_path)[1] == ".mojom"
|
||||
|
||||
|
||||
def CheckNoUpstreamDepsOnClank(input_api, output_api):
|
||||
"""Prevent additions of dependencies from the upstream repo on //clank."""
|
||||
# clank can depend on clank
|
||||
@ -2033,6 +2037,30 @@ def CheckNoDEPSGIT(input_api, output_api):
|
||||
return []
|
||||
|
||||
|
||||
def CheckCrosApiNeedBrowserTest(input_api, output_api):
|
||||
"""Check new crosapi should add browser test."""
|
||||
has_new_crosapi = False
|
||||
has_browser_test = False
|
||||
for f in input_api.AffectedFiles():
|
||||
path = f.LocalPath()
|
||||
if (path.startswith('chromeos/crosapi/mojom') and
|
||||
_IsMojomFile(input_api, path) and f.Action() == 'A'):
|
||||
has_new_crosapi = True
|
||||
if path.endswith('browsertest.cc') or path.endswith('browser_test.cc'):
|
||||
has_browser_test = True
|
||||
if has_new_crosapi and not has_browser_test:
|
||||
return [
|
||||
output_api.PresubmitPromptWarning(
|
||||
'You are adding a new crosapi, but there is no file ends with '
|
||||
'browsertest.cc file being added or modified. It is important '
|
||||
'to add crosapi browser test coverage to avoid version '
|
||||
' skew issues.\n'
|
||||
'Check //docs/lacros/test_instructions.md for more information.'
|
||||
)
|
||||
]
|
||||
return []
|
||||
|
||||
|
||||
def CheckValidHostsInDEPSOnUpload(input_api, output_api):
|
||||
"""Checks that DEPS file deps are from allowed_hosts."""
|
||||
# Run only if DEPS file has been modified to annoy fewer bystanders.
|
||||
|
@ -4412,6 +4412,55 @@ class CheckDeprecationOfPreferencesTest(unittest.TestCase):
|
||||
'Broken .*MIGRATE_OBSOLETE_.*_PREFS markers in browser_prefs.cc.',
|
||||
errors[0].message)
|
||||
|
||||
class CheckCrosApiNeedBrowserTestTest(unittest.TestCase):
|
||||
def testWarning(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.files = [
|
||||
MockAffectedFile('chromeos/crosapi/mojom/example.mojom', [], action='A'),
|
||||
]
|
||||
result = PRESUBMIT.CheckCrosApiNeedBrowserTest(mock_input_api, mock_output_api)
|
||||
self.assertEqual(1, len(result))
|
||||
self.assertEqual(result[0].type, 'warning')
|
||||
|
||||
def testNoWarningWithBrowserTest(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.files = [
|
||||
MockAffectedFile('chromeos/crosapi/mojom/example.mojom', [], action='A'),
|
||||
MockAffectedFile('chrome/example_browsertest.cc', [], action='A'),
|
||||
]
|
||||
result = PRESUBMIT.CheckCrosApiNeedBrowserTest(mock_input_api, mock_output_api)
|
||||
self.assertEqual(0, len(result))
|
||||
|
||||
def testNoWarningModifyCrosapi(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.files = [
|
||||
MockAffectedFile('chromeos/crosapi/mojom/example.mojom', [], action='M'),
|
||||
]
|
||||
result = PRESUBMIT.CheckCrosApiNeedBrowserTest(mock_input_api, mock_output_api)
|
||||
self.assertEqual(0, len(result))
|
||||
|
||||
def testNoWarningAddNonMojomFile(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.files = [
|
||||
MockAffectedFile('chromeos/crosapi/mojom/example.cc', [], action='A'),
|
||||
]
|
||||
result = PRESUBMIT.CheckCrosApiNeedBrowserTest(mock_input_api, mock_output_api)
|
||||
self.assertEqual(0, len(result))
|
||||
|
||||
def testNoWarningNoneRelatedMojom(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.files = [
|
||||
MockAffectedFile('random/folder/example.mojom', [], action='A'),
|
||||
]
|
||||
result = PRESUBMIT.CheckCrosApiNeedBrowserTest(mock_input_api, mock_output_api)
|
||||
self.assertEqual(0, len(result))
|
||||
|
||||
|
||||
class MPArchApiUsage(unittest.TestCase):
|
||||
def _assert_notify(
|
||||
self, expected_uses, expect_fyi, msg, local_path, new_contents):
|
||||
|
Reference in New Issue
Block a user