Add presubmit to warn when Layout() is called within a test.
View::Layout() is getting an overhaul. Direct calls to Layout() from within tests and eventually production code will be forbidden. There are new test-only functions available to manually invoke a layout pass properly.
This presubmit will attempt catch direct calls to Layout() from within files ending in unittest, browsertest, or ui_test. It should still allow Layout() overrides. While this presubmit should not trigger for existing calls to Layout(), those calls will have been eliminated in other CLs.
To manually test this presubmit:
> git cl presubmit --force --files path_to/some/test/foo_unittest.cc
This was tested with and without the Layout() call and was confirmed to trigger when the line of code with the Layout() call was added or changed.
Change-Id: Ib784203e7f795cc4a437508d4c37aecf8f90d6fd
Bug: 1350521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3899633
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1050232}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 3caa4211..0d879d8 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1757,7 +1757,7 @@
def _GetMessageForMatchingType(input_api, affected_file, line_number, line,
ban_rule):
- """Helper method for CheckNoBannedFunctions and CheckNoDeprecatedMojoTypes.
+ """Helper method for checking for banned constructs.
Returns an string composed of the name of the file, the line number where the
match has been found and the additional text passed as |message| in case the
@@ -1879,6 +1879,34 @@
'\n'.join(errors)))
return result
+def CheckNoLayoutCallsInTests(input_api, output_api):
+ """Make sure there are no explicit calls to View::Layout() in tests"""
+ warnings = []
+ ban_rule = BanRule(
+ r'/(\.|->)Layout\(\);',
+ (
+ 'Direct calls to View::Layout() are not allowed in tests. '
+ 'If the view must be laid out here, use RunScheduledLayout(view). It '
+ 'is found in //ui/views/test/views_test_utils.h. '
+ 'See http://crbug.com/1350521 for more details.',
+ ),
+ False,
+ )
+ file_filter = lambda f: input_api.re.search(
+ r'_(unittest|browsertest|ui_test).*\.(cc|mm)$', f.LocalPath())
+ for f in input_api.AffectedFiles(file_filter = file_filter):
+ for line_num, line in f.ChangedContents():
+ problems = _GetMessageForMatchingType(input_api, f,
+ line_num, line,
+ ban_rule)
+ if problems:
+ warnings.extend(problems)
+ result = []
+ if (warnings):
+ result.append(
+ output_api.PresubmitPromptWarning(
+ 'Banned call to View::Layout() in tests.\n\n'.join(warnings)))
+ return result
def _CheckAndroidNoBannedImports(input_api, output_api):
"""Make sure that banned java imports are not used."""
