Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2477813002:
Fixing how --gtest-filter-file handles negative patterns. (Closed)
|
Created:
4 years, 1 month ago by Łukasz Anforowicz Modified:
4 years, 1 month ago Reviewers:
jbudorick CC:
chromium-reviews, mikecase+watch_chromium.org, jbudorick+watch_chromium.org, agrieve+watch_chromium.org, alexmos, site-isolation-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixing how --gtest-filter-file handles negative patterns.
When authoring the README.md file in https://crrev.com/2472153002, I've
noticed that test_runner.py doesn't properly handle negative patterns
from the filter files. This CL fixes this and adds some unit tests
around this. The changes are based on the understanding of the desired
format of --gtest_filter that is based on:
1) the format used in chromium.fyi.json prior to introducing
testing/buildbot/filters directory - for example notice multiple
patterns, but only a single '-' character in:
https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a596459fdf75ebad9/testing/buildbot/chromium.fyi.json#5690
2) gtest documentation at
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-a-subset-of-the-tests
BUG=654569
Committed: https://crrev.com/1f3f8c66d6efc3e759550eaa899757fc34307cfc
Cr-Commit-Position: refs/heads/master@{#430634}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Converted a comment into a doc string. #
Depends on Patchset: Messages
Total messages: 23 (16 generated)
lukasza@chromium.org changed reviewers: + jbudorick@chromium.org
jbudorick@, can you please take a look?
Description was changed from ========== Fixing how --gtest-filter-file handles negative patterns. When authoring the README.md file in https://crrev.com/2472153002, I've noticed that test_runner.py doesn't properly handle negative patterns from the filter files. This CL fixes this and adds some unit tests around this. The changes are based on the understanding of the desired format of --gtest_filter that is based on: 1) the format used in chromium.fyi.json prior to introducing testing/buildbot/filters directory - for example notice multiple patterns, but only a single '-' character in: https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a5964... 2) gtest documentation at https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid... BUG= ========== to ========== Fixing how --gtest-filter-file handles negative patterns. When authoring the README.md file in https://crrev.com/2472153002, I've noticed that test_runner.py doesn't properly handle negative patterns from the filter files. This CL fixes this and adds some unit tests around this. The changes are based on the understanding of the desired format of --gtest_filter that is based on: 1) the format used in chromium.fyi.json prior to introducing testing/buildbot/filters directory - for example notice multiple patterns, but only a single '-' character in: https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a5964... 2) gtest documentation at https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid... BUG=654569 ==========
lgtm w/ nit Thanks for this & for writing that doc. https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... File build/android/pylib/gtest/gtest_test_instance.py (right): https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... build/android/pylib/gtest/gtest_test_instance.py:201: # Converts file contents described by //testing/buildbot/filters/README.md Nit: make this a docstring, please.
Thanks for reviewing. https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... File build/android/pylib/gtest/gtest_test_instance.py (right): https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... build/android/pylib/gtest/gtest_test_instance.py:201: # Converts file contents described by //testing/buildbot/filters/README.md On 2016/11/03 18:21:18, jbudorick wrote: > Nit: make this a docstring, please. Ooops. Thanks for catching this. I guess I've been doing too much C++ and too little Python recently :-) The docstring turned out to be relatively long, but I think this is okay.
On 2016/11/03 19:34:39, Łukasz Anforowicz wrote: > Thanks for reviewing. > > https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... > File build/android/pylib/gtest/gtest_test_instance.py (right): > > https://codereview.chromium.org/2477813002/diff/1/build/android/pylib/gtest/g... > build/android/pylib/gtest/gtest_test_instance.py:201: # Converts file contents > described by //testing/buildbot/filters/README.md > On 2016/11/03 18:21:18, jbudorick wrote: > > Nit: make this a docstring, please. > > Ooops. Thanks for catching this. I guess I've been doing too much C++ and too > little Python recently :-) > > The docstring turned out to be relatively long, but I think this is okay. Agreed, looks fine to me.
The CQ bit was checked by lukasza@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lukasza@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by lukasza@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by lukasza@chromium.org
The CQ bit was checked by lukasza@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/2477813002/#ps20001 (title: "Converted a comment into a doc string.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fixing how --gtest-filter-file handles negative patterns. When authoring the README.md file in https://crrev.com/2472153002, I've noticed that test_runner.py doesn't properly handle negative patterns from the filter files. This CL fixes this and adds some unit tests around this. The changes are based on the understanding of the desired format of --gtest_filter that is based on: 1) the format used in chromium.fyi.json prior to introducing testing/buildbot/filters directory - for example notice multiple patterns, but only a single '-' character in: https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a5964... 2) gtest documentation at https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid... BUG=654569 ========== to ========== Fixing how --gtest-filter-file handles negative patterns. When authoring the README.md file in https://crrev.com/2472153002, I've noticed that test_runner.py doesn't properly handle negative patterns from the filter files. This CL fixes this and adds some unit tests around this. The changes are based on the understanding of the desired format of --gtest_filter that is based on: 1) the format used in chromium.fyi.json prior to introducing testing/buildbot/filters directory - for example notice multiple patterns, but only a single '-' character in: https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a5964... 2) gtest documentation at https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid... BUG=654569 Committed: https://crrev.com/1f3f8c66d6efc3e759550eaa899757fc34307cfc Cr-Commit-Position: refs/heads/master@{#430634} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/1f3f8c66d6efc3e759550eaa899757fc34307cfc Cr-Commit-Position: refs/heads/master@{#430634} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
