Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1856253002:
Change PRESUBMIT.py to make sure the right FlakyTest being used (Closed)
|
Created:
4 years, 8 months ago by Yoland Yan(Google) Modified:
4 years, 8 months ago Reviewers:
rnephew (Wrong account), Paweł Hajdan Jr., mikecase (-- gone --), jbudorick, rnephew (Reviews Here) CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChange PRESUBMIT.py to make sure the right FlakyTest being used
BUG=602361
Committed: https://crrev.com/daabc6db764177bd31d9ade06be383a4aa3e6617
Cr-Commit-Position: refs/heads/master@{#387960}
Patch Set 1 #
Total comments: 10
Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #
Total comments: 2
Patch Set 5 : #Messages
Total messages: 21 (9 generated)
yolandyan@google.com changed reviewers: + jbudorick@chromium.org
yolandyan@google.com changed reviewers: + mikecase@chromium.org, rnephew@google.com
Description was changed from ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG= ========== to ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG=602361 ==========
yolandyan@google.com changed reviewers: + phajdan.jr@chromium.org
non-owner lgtm
rnephew@chromium.org changed reviewers: + rnephew@chromium.org
https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode484 PRESUBMIT.py:484: return [ output_api.PresubmitError( nit: [output_api... https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode485 PRESUBMIT.py:485: 'Use org.chromium.base.test.util.FlakyTest instead of ' + nit: 4 space indentation. no need for + here. This could just be: 'Use org.chromium.base.test.util.FlakyTest instead of ' 'android.test.FlakyTest', https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode487 PRESUBMIT.py:487: files) ] nit: files)]
https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode479 PRESUBMIT.py:479: if (f.LocalPath().endswith('Test.java')): nit: if (f.LocalPath().endswith('Test.java')): into: if f.LocalPath().endswith('Test.java'): https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode481 PRESUBMIT.py:481: if pattern.search(contents): Since contents isn't used elsewhere, you could probably just do: if pattern.search(input_api.ReadFile(f)):
Shame on me https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode479 PRESUBMIT.py:479: if (f.LocalPath().endswith('Test.java')): On 2016/04/11 18:58:26, rnephew1 wrote: > nit: > if (f.LocalPath().endswith('Test.java')): > into: > if f.LocalPath().endswith('Test.java'): > Done. https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode481 PRESUBMIT.py:481: if pattern.search(contents): On 2016/04/11 18:58:26, rnephew1 wrote: > Since contents isn't used elsewhere, you could probably just do: > if pattern.search(input_api.ReadFile(f)): Done. https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode484 PRESUBMIT.py:484: return [ output_api.PresubmitError( On 2016/04/11 18:56:30, rnephew1 wrote: > nit: [output_api... Done. https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode485 PRESUBMIT.py:485: 'Use org.chromium.base.test.util.FlakyTest instead of ' + On 2016/04/11 18:56:30, rnephew1 wrote: > nit: 4 space indentation. > > no need for + here. This could just be: > 'Use org.chromium.base.test.util.FlakyTest instead of ' > 'android.test.FlakyTest', Done. https://codereview.chromium.org/1856253002/diff/1/PRESUBMIT.py#newcode487 PRESUBMIT.py:487: files) ] On 2016/04/11 18:56:30, rnephew1 wrote: > nit: files)] Done.
LGTM w/comment https://codereview.chromium.org/1856253002/diff/60001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1856253002/diff/60001/PRESUBMIT.py#newcode488 PRESUBMIT.py:488: if pattern.search(input_api.ReadFile(f)): nit: Indent this just +2, not +4.
https://codereview.chromium.org/1856253002/diff/60001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/1856253002/diff/60001/PRESUBMIT.py#newcode488 PRESUBMIT.py:488: if pattern.search(input_api.ReadFile(f)): On 2016/04/12 20:43:31, Paweł Hajdan Jr. wrote: > nit: Indent this just +2, not +4. Done.
The CQ bit was checked by yolandyan@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org, phajdan.jr@chromium.org Link to the patchset: https://codereview.chromium.org/1856253002/#ps80001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1856253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1856253002/80001
Message was sent while issue was closed.
Description was changed from ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG=602361 ========== to ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG=602361 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG=602361 ========== to ========== Change PRESUBMIT.py to make sure the right FlakyTest being used BUG=602361 Committed: https://crrev.com/daabc6db764177bd31d9ade06be383a4aa3e6617 Cr-Commit-Position: refs/heads/master@{#387960} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/daabc6db764177bd31d9ade06be383a4aa3e6617 Cr-Commit-Position: refs/heads/master@{#387960} |
