The InputApi mock was using re.search() instead of re.match(). This
meant that some tests in PRESUBMIT_tests.py would pass while the regex
wasn't working on production.
Change-Id: I5225afaa44602bfb0d8d16a1a4d0d021e1e81bb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3302819
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/main@{#946145}
This CL migrates the root-level PRESUBMIT checks, their associated
tests, and and one other file that got pulled along for the ride)
to run under Python3 instead of Python2.
Bug: 816629
Change-Id: I75f3edb34ca72458432ba54f3afa022e027f8eb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2877897
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#883002}
The //PRESUBMIT.py checks may sometimes be run on a bot that is actually
testing changes to a repo other than chromium src (e.g., v8 or ANGLE
or something). Sometimes it makes sense to run the presubmit changes
anyway, but the inclusive language check should only be run on actual
changes to chromium/src, since that's what the allowlist checks against.
This CL tweaks the check to exit early on changes for other repos, and
removes the temporary workaround we added earlier today in
f0b0b46c/#{868614).
Bug: 1194996
Change-Id: I0332768a655528e0de0fd52c63ee17ebc15b50e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2801150
Reviewed-by: Sean McCullough <seanmccullough@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#869219}
It's a common mistake that developers just remove Register...Pref()
calls for preferences they don't need anymore. If they do this, the
preference stays in the prefs files on disk for ever. A proper approach
is to first ClearPref() the preference for some releases and then to
remove the code.
This CL introduces a presubmit warning if we detect that a
Register...Pref() call is removed from a non-unittest.
Bug: 1153014
Change-Id: If8f19933de039553ef47e0ddce12eb194df45ce1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560205
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833730}
MockInputApi was returning MockFile objects instead of strings
from LocalPaths() API, which apparently trips up Windows Python
APIs, but not Unix ones.
Bug: 984360
Change-Id: I0a14cc07a9406d84d6b20d104d002fe67cc4ae86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1706577
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678454}
The grd reader is invoked from _GetGrdMessages with defines which
specify "chromium" build.
Drive-by fix for the MockFile to support basename on Windows.
BUG=984910
TEST=A modified grd fail doesn't trip the presubmit run.
Change-Id: I1b30dcaf8b91d40292e631ba854b4f0dfa54de69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1705799
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678407}
Parameters |white_list| and |black_list| in FilterSourceFile()
should be iterables, and it's fairly easy to wrongly pass a string
instead.
This checking caught _CheckCrbugLinksHaveHttps in unit tests.
Bug: 869103
Change-Id: I1cd6d62c3fa2b1500d9d7b6b35794f40e35378af
Reviewed-on: https://chromium-review.googlesource.com/1155379
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579288}
The root PRESUBMIT.py contains
_CheckNoProductionCodeUsingTestOnlyFunctions which checks against
including a call to a 'for testing only' method in production code.
That check has not been tested. This CL adds two tests for it.
Additionally, the CL also fixes two issues in the presubmit tests:
* FilterSourceFile in PRESUBMIT_test_mocks.py did not match the
production version from tools/depot_tools/presubmit_support.py: the
production requires the filename to be in the whitelist, while the
mock version was OK with just not being in the blacklist. The CL
modifies the mock version to match the production.
* The test for _CheckAndroidTestJUnitInheritance did not adhere to
the whitelisted filename pattern (*Test.java). This CL changes the
names of the fake files to match the pattern.
Bug: 821981
Change-Id: I65edf07ddb2ae26ad7d08ceb7cf4d51b482b5e56
Reviewed-on: https://chromium-review.googlesource.com/966605
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543783}
The testing::Test fixture (used by TEST macro) does not drain the
autorelease pool after a test. PlatformTest should be used.
Add a PRESUBMIT check that neither TEST nor testing::Test is used
in iOS Objective-C++ test files. Files are assumed to be iOS if
either their base name match '\bios\b' or one of the component in
the path is 'ios'.
Expand MockInputApi to filter files in mocks of AffectedFiles and
AffectedSourceFiles function, adding missing mocked functions too.
Fix unit tests that were failing after the filtering is correctly
implemented.
Bug: none
Change-Id: I0af99b6658b8e15888dfcfb94345eb879ab9fd37
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/937204
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539829}
This CL omits tests for directories where dependencies aren't checked:
PRESUBMIT test mocks is missing support for FileSourceFilter. This will
be added to test mocks along with the missing tests in a followup CL.
Bug: 763980
Change-Id: Ibf9265911f32d24f446e690d9ec05ddb7a61f198
Reviewed-on: https://chromium-review.googlesource.com/671812
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505189}
These presubmit warnings prevent people from adding new JUnit3 tests or
use inheritance in JUnit4 testing, it also prevent people from using
the deprecated JUnit framework.
For more on JUnit4 migration, please check
//testing/android/docs/junit4.md
Bug: 640116
Change-Id: I941b595f6fbc01ac60a2647ab0af64482596d9cc
Reviewed-on: https://chromium-review.googlesource.com/634603
Commit-Queue: Yoland Yan <yolandyan@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497790}
Since mojo manifests don't have a consistent naming convention, this
simply looks for JSON files with the "interface_provider_specs" key:
there are some manifests that don't specify this, but the important
part for security review is auditing what's exposed between processes.
Bug: 695922
Change-Id: Id30dae51ecc0cbfa35650ead14ef2dfd081c23d7
Reviewed-on: https://chromium-review.googlesource.com/621707
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497410}
Restoring test to ensure that document.getElementById() calls are flagged as
style violations, now that these checks are performed via ESLint. These tests
are not meant to be exhaustive, instead they just validate that our PRESUBMIT
integration with ESLint works.
BUG=720034
Review-Url: https://codereview.chromium.org/2917473002
Cr-Commit-Position: refs/heads/master@{#476014}
Skip presubmit warning when the introduced useless forward
declaration is made in third_party (with the exception of
blink). Take the opportunity to fix one test and style.
BUG=662195
TEST=PRESUBMIT_test.py ForwardDeclarationTest
Review-Url: https://codereview.chromium.org/2568473002
Cr-Commit-Position: refs/heads/master@{#437833}
Checks that added or removed lines in affected header files
do not lead to new useless class or struct forward declaration.
BUG=662195
TEST=PRESUBMIT_test.py ForwardDeclarationTest
Review-Url: https://codereview.chromium.org/2532583002
Cr-Commit-Position: refs/heads/master@{#434449}
On Windows this type logs to the Event Log and on POSIX systems it logs to the messages log (or its equvalent).
As a side effect of adding the presubmit check for it this
CL fixes running the presumbit checks tests on Windows.
BUG=642115
Review-Url: https://codereview.chromium.org/2296783002
Cr-Commit-Position: refs/heads/master@{#419758}
Have the presubmit script check all affected files instead of only
those in the presubmit directory. Avoids false positives.
BUG=469920
Review-Url: https://codereview.chromium.org/1925273002
Cr-Commit-Position: refs/heads/master@{#390968}
Now unconditionally including some directory trees to capture imports that differ based on python version (at least those that have come up so far).
BUG=589318, 599692
Review URL: https://codereview.chromium.org/1846103005
Cr-Commit-Position: refs/heads/master@{#384907}
Now with check disabled for non-android checkouts.
This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).
BUG=589318
Review URL: https://codereview.chromium.org/1840113002
Cr-Commit-Position: refs/heads/master@{#384039}
This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).
BUG=589318
Review URL: https://codereview.chromium.org/1784373002
Cr-Commit-Position: refs/heads/master@{#383127}
Add a presubmit script that checks for when extension api files are touched, but
the corresponding extern file is not. Right now, this is very simple - it only
checks that the extern is modified in some way (doesn't validate that it's the
*right* way), and is only a warning (because sometimes api file changes don't
cause extern changes). As an improvement, we would validate that the extern
files contain the proper content - let's do that later.
This will also be rolled out piecemeal, since many APIs don't currently have a
dedicated extern file. This change only imposes the check on bluetooth (because
it was handy) - if all goes well, we'll roll this out to all api files.
Also establish a dedicated extern folder, since it's silly for chrome-generated
externs to live in third_party/.
BUG=469920
Review URL: https://codereview.chromium.org/1718243003
Cr-Commit-Position: refs/heads/master@{#378018}
Presubmit does not contain any tests for the _CheckUserActionUpdate
function. This commit adds a test to check against an action in
actions.xml and one that is not found there. To do this we have to
improve the MockInputApi and MockFile classes.
BUG=462814
Review URL: https://codereview.chromium.org/966703003
Cr-Commit-Position: refs/heads/master@{#318759}
Also added a testcase for it and a nice print of errors.
BUG=443026
Review URL: https://codereview.chromium.org/809053003
Cr-Commit-Position: refs/heads/master@{#317794}
Add a presubmit check that warns about declaring Singleton<T> in header files
(except for base/memory/singleton.h)
This check is copied from presubmit_canned_checks.py in depot_tools/ with an
added exception for base/memory/singleton.h.
The corresponding code will be removed from presubmit_canned_checks.py afterwards.
BUG=349861
Review URL: https://codereview.chromium.org/933253002
Cr-Commit-Position: refs/heads/master@{#316852}
Reason for revert:
Reverting because of presubmit crashes:
Traceback (most recent call last):
File "/usr/local/google/ssd/depot_tools/git_cl.py", line 2983, in <module>
sys.exit(main(sys.argv[1:]))
File "/usr/local/google/ssd/depot_tools/git_cl.py", line 2969, in main
return dispatcher.execute(OptionParser(), argv)
File "/usr/local/google/ssd/depot_tools/subcommand.py", line 245, in execute
return command(parser, args[1:])
File "/usr/local/google/ssd/depot_tools/git_cl.py", line 1614, in CMDpresubmit
change=cl.GetChange(base_branch, None))
File "/usr/local/google/ssd/depot_tools/git_cl.py", line 936, in RunHook
rietveld_obj=self.RpcServer())
File "/usr/local/google/ssd/depot_tools/presubmit_support.py", line 1396, in DoPresubmitChecks
results += executer.ExecPresubmitScript(presubmit_script, filename)
File "/usr/local/google/ssd/depot_tools/presubmit_support.py", line 1313, in ExecPresubmitScript
result = eval(function_name + '(*__args)', context)
File "<string>", line 1, in <module>
File "<string>", line 1727, in CheckChangeOnCommit
File "<string>", line 1455, in _CommonChecks
File "<string>", line 1322, in _CheckSingletonInHeaders
File "/usr/local/google/ssd/depot_tools/presubmit_support.py", line 472, in ReadFile
raise IOError('Access outside the repository root is denied.')
IOError: Access outside the repository root is denied.
Original issue's description:
> Add a presubmit check that warns about declaring Singleton<T> in header files
> (except for base/memory/singleton.h)
> This check is copied from presubmit_canned_checks.py in depot_tools/ with an
> added exception for base/memory/singleton.h.
> The corresponding code will be removed from presubmit_canned_checks.py afterwards.
>
> BUG=349861
> R=phajdan.jr@chromium.org,
>
> Committed: https://crrev.com/9b73d0393037452d13358f4e2766cb62adbdede8
> Cr-Commit-Position: refs/heads/master@{#316546}
TBR=phajdan.jr@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=349861
Review URL: https://codereview.chromium.org/934723003
Cr-Commit-Position: refs/heads/master@{#316559}
(except for base/memory/singleton.h)
This check is copied from presubmit_canned_checks.py in depot_tools/ with an
added exception for base/memory/singleton.h.
The corresponding code will be removed from presubmit_canned_checks.py afterwards.
BUG=349861
R=phajdan.jr@chromium.org,
Review URL: https://codereview.chromium.org/929043002
Cr-Commit-Position: refs/heads/master@{#316546}
In case actions.xml is included in the changelist the chrome/browser/resources/PRESUBMIT.py shouldn't run because the presubmit for actions.xml will do more complete job.
Fixing the bug to look into all changed files in the CL where previously it was only looking at subdirectories of current presubmit script to find actions.xml
BUG=439978
Review URL: https://codereview.chromium.org/760163004
Cr-Commit-Position: refs/heads/master@{#308244}
This presubmit task will catch the case when HTML files are modified with new user action and action.xml is not updated
BUG=431393
Review URL: https://codereview.chromium.org/719463003
Cr-Commit-Position: refs/heads/master@{#306842}
