This reverts commit 74e5e087cb.
Reason for reland: Fixed blink & webapk presubmit tests
Original change's description:
> Revert "Improve mocking of os_path.exists() in PRESUBMIT_test.py"
>
> This reverts commit b35a9a9465.
>
> Reason for revert: Manual bisect shows that this is responsible for failures starting in https://ci.chromium.org/ui/p/chromium/builders/ci/linux-presubmit/19822/blamelist. Local repro on Linux: `vpython3 chrome/android/webapk/PRESUBMIT_test.py --verbose`
>
> Original change's description:
> > Improve mocking of os_path.exists() in PRESUBMIT_test.py
> >
> > I need this for an upcoming change, but it turns out it found 2 real
> > bugs:
> > * A .pydeps check that would never be hit due to test mocks returning
> > that deleted files exist
> > * A translations check that would never be hit due to test mocks using
> > local paths when absolute paths were requested.
> >
> > Bug: None
> > Change-Id: I920e4f67b2ff1193b89c7abfd9664d14a94c44bf
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5910106
> > Reviewed-by: Ted Choc <tedchoc@chromium.org>
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1364918}
>
> Bug: None
> Change-Id: Ibcc1931e5709d1a4209e56211595df8b94a35c9a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5913697
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
> Owners-Override: Łukasz Anforowicz <lukasza@chromium.org>
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1364999}
Bug: None
Change-Id: I303d4d1abc685e3bad854e9a5ebe8ae2efe48869
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5912982
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Owners-Override: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1365664}
I need this for an upcomping change, but it turns out it found 2 real
bugs:
* A .pydeps check that would never be hit due to test mocks returning
that deleted files exist
* A translations check that would never be hit due to test mocks using
local paths when absolute paths were requested.
Bug: None
Change-Id: I920e4f67b2ff1193b89c7abfd9664d14a94c44bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5910106
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1364918}
Implement a presubmit check in c/b/PRESUBMIT.py that scans changes
to BUILD.gn files under a set of Ash directories. It generates a
warning for each such BUILD.gn files to which source entries were
added that contain a slash, thus e.g. denoting a file in a subdirectory.
For example, if a new file c/b/ash/foo/BUILD.gn is added containing
static_library("foo") {
sources = [ "bar/baz.cc", "x.cc", "../bla.h" ]
...
}
then the check will warn about bar/baz.cc and ../bla.h.
(The parsing is done via regexps and is not 100% precise.)
This presubmit check is meant to protect against regressions in the
context of the ongoing Ash refactoring project (b/332804822) where we
are creating BUILD.gn files for subdirectories.
Bug: b:357772015
Change-Id: Ibb7b99c2531bc129fd06ef14cd7ed07a361be33a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5768197
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1345766}
In Python 3.11, 'U' ("universal newline") is no longer accepted in
the file mode, having been deprecated in Python 3.3. The "universal
newline" is used by default when a file is open in text mode.
This commit removes the 'U' from the two (non-third-party) places
it is used.
Bug: 1357549
Change-Id: I3305707858d8ba7a9f518656a9b97dc1702bbe94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3859535
Reviewed-by: Mike Pinkerton <pinkerton@chromium.org>
Commit-Queue: Joanmarie Diggs <jdiggs@igalia.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1040794}
When I read the Chromium source, I first noticed that some python files for presubmit located in the root directory had some problems (like incorrect function names, indentation, typos, etc.), so I tried to fix them.
BUG=nobug
Change-Id: I3646b9fc1761d3cd2af884aba8d815d09665e7be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3812981
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Erik Staab <estaab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032601}
CheckAddedDepsHaveTargetApprovals and CheckStableMojomChanges only look
at lines that have changed. This makes them expensive (OldContents() is
one of the most expensive presubmit functions) and guarantees that they
will not find anything when run with --all or --files, when there are no
changed lines.
Therefore this change pragmatically returns early from these functions
when no_diffs is true. This reduces "presubmit --all" times on my test
machine by about 6 minutes, or 5-10%, allowing for faster iteration
times.
Bug: 1309977
Change-Id: I3a947efa6a59d562aa27f80fdda3f7ad38993352
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3671444
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
Cr-Commit-Position: refs/heads/main@{#1010839}
When running "git cl presubmit --all" RunEsLintChecks may be called with
~1,100 or more arguments. This leads to a command line that is too long
for Windows to handle. This change invokes eslint multiple times to
avoid hitting the limit.
This change was initially landed as crrev.com/c/3554791 but that change
used the wrong return type and was ultimately done at the wrong level in
the call hierarchy. RunEsLintChecks is the right place to do this change
because it has the list of affected files as a discrete list.
This change can be (and has been) tested with this command (using the
recently added --files option) to make sure it runs to completion:
git cl presubmit --files=*.js --force
For much faster results return early from PanProjectChecks in
depot_tools/presubmit_canned_checks.py.
See crrev.com/c/3554633 for another example of fixing this issue.
This change also adds is_windows to the MockInputAPI - it exists in the
real InputAPI class.
Bug: 1309977, 1312819
Change-Id: Ib2e0bb287b4050b09206b1d2c7155bbc519a4b34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572912
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989961}
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}
