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 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(-)
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 removed Daniel Cheng from this change.
To view, visit change 966906. To unsubscribe, or for help writing mail filters, visit settings.
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.
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.
Patch set 7:Code-Review +1
1 comment:
Patch Set #7, Line 2639: full_path = input_api.PresubmitLocalPath() + '/' + path
Nit: 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.
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.
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 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(-)