Run PRESUBMIT_test.py in subdirectories [chromium/src : master]
Vaclav Brozek (Gerrit)
Just an update on patch set 4 (+ the automated 5, 6): I added a shebang header and executable bit for all PRESUBMIT_test.py which did not have it already. Otherwise, presubmit will fail to run those tests if it decides it's necessary.
Patch set 6:Commit-Queue +1
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Vaclav Brozek (Gerrit)
Vaclav Brozek would like Jochen Eisinger to review this change.
Run PRESUBMIT_test.py in subdirectories
Right now, changes to //PRESUBMIT.py trigger running
//PRESUBMIT_test.py during presubmit checks.
However, this does not happen for PRESUBMITs in subdirectories, of
which there is a number with associated PRESUBMIT_test. This CL allows
running the associated PRESUBMIT_test.
Bug: 821981
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I057f4d736992795dde7dd0d329ed3773d22553dc
---
M PRESUBMIT.py
M chrome/browser/resources/PRESUBMIT_test.py
M ios/PRESUBMIT_test.py
M media/PRESUBMIT_test.py
M third_party/WebKit/PRESUBMIT_test.py
5 files changed, 12 insertions(+), 5 deletions(-)
Vaclav Brozek (Gerrit)
So, actually, let me distribute my CLs in this area a bit more evenly, since Daniel already reviewed two PRESUBMIT CLs from me recently.
Jochen, could you have a look at this one?
Thanks!
Vaclav
Vaclav Brozek (Gerrit)
Vaclav Brozek removed Daniel Cheng from this change.
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
1 comment:
Patch Set #6, Line 2636:
match = test_pattern.search(f.LocalPath())Would it be possible to write this as
path, name = input_api.os_path.split(f.LocalPath())
if name == 'PRESUBMIT.py':
results.extend(input_api.canned_checks.RunUnitTestsInDirectory(...))
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Vaclav Brozek (Gerrit)
Thanks, Daniel.
Please have a look at the new patch set.
Given that you started reviewing it anyway, I'm switching back to you for review. Sorry both to you and Jochen for this back and forth.
Cheers,
Vaclav
1 comment:
Patch Set #6, Line 2636:
for f in input_api.AffectedFiles():Would it be possible to write this as […]
Sure, thanks for pointing that out! With os.path.split it is much clearer what the code does and that it does it correctly than with the regexp.
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Patch set 7:Code-Review +1
1 comment:
Patch Set #7, Line 2639:
full_path = input_api.PresubmitLocalPath() + '/' + pathNit: if input_api.os_path.join works here, let's use that
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Jochen Eisinger (Gerrit)
Vaclav Brozek (Gerrit)
Thanks both for the review!
Vaclav
Patch set 8:Commit-Queue +2
1 comment:
Patch Set #7, Line 2639:
full_path = input_api.os_path.join(input_api.PresubmitLocalPath(), path)Nit: if input_api.os_path. […]
Good point!
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"os.path.join" https://chromium-review.googlesource.com/c/966906/8
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/966906/8
Bot data: {"action": "start", "triggered_at": "2018-03-20T09:11:17.0Z", "cq_cfg_revision": "5b6c43e4d6b0297aa92e118e785d640c42297271", "revision": "e2ebc7c7485d3b63ea5f0cea8114daf5d326e8f9"}
Commit Bot (Gerrit)
Commit Bot merged this change.
Run PRESUBMIT_test.py in subdirectories
Right now, changes to //PRESUBMIT.py trigger running
//PRESUBMIT_test.py during presubmit checks.
However, this does not happen for PRESUBMITs in subdirectories, of
which there is a number with associated PRESUBMIT_test. This CL allows
running the associated PRESUBMIT_test.
Bug: 821981
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I057f4d736992795dde7dd0d329ed3773d22553dc
Reviewed-on: https://chromium-review.googlesource.com/966906
Commit-Queue: Vaclav Brozek <va...@chromium.org>
Reviewed-by: Daniel Cheng <dch...@chromium.org>
Reviewed-by: Jochen Eisinger <joc...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544318}
---
M PRESUBMIT.py
M chrome/browser/resources/PRESUBMIT_test.py
M ios/PRESUBMIT_test.py
M media/PRESUBMIT_test.py
M third_party/WebKit/PRESUBMIT_test.py
5 files changed, 11 insertions(+), 5 deletions(-)