Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2171103002:
Add test for WebBluetooth chooser spinner, status text and rescan button (Closed)
|
Created:
4 years, 5 months ago by juncai Modified:
4 years, 4 months ago CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd test for WebBluetooth chooser spinner, status text and rescan button
This is part 5 of matching WebBluetooth chooser views to mocks.
This CL adds test code for spinner, status text and rescan button that
was added in previous patches.
BUG=583452
Committed: https://crrev.com/68973c4f29e3cfdee4edaca1768c0db9635190b7
Cr-Commit-Position: refs/heads/master@{#408247}
Patch Set 1 : added test for WebBluetooth chooser spinner, status text and rescan button #
Total comments: 35
Patch Set 2 : address comments #
Total comments: 4
Patch Set 3 : moved throbber related tests from browser test to unit test #Patch Set 4 : removed unnecessary comments #Patch Set 5 : changed chooser_dialog_view_browsertest.cc to chooser_dialog_view_unittest.cc #
Total comments: 4
Patch Set 6 : address comments #Dependent Patchsets: Messages
Total messages: 39 (26 generated)
The CQ bit was checked by juncai@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...
juncai@chromium.org changed reviewers: + msw@chromium.org, reillyg@chromium.org
msw@chromium.org, please review changes in: //chrome/browser/ui/views/ reillyg@chromium.org, please review changes in: //chrome/browser/chooser_controller/
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... File chrome/browser/chooser_controller/mock_chooser_controller.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:48: } nit: no braces https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:57: } nit: no braces
https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... File chrome/browser/chooser_controller/mock_chooser_controller.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:40: void MockChooserController::OnAdapterPresenceChanged(AdapterPresence presence) { It'd be nice if the non-test code used the same enums (AdapterPresence and DiscoveryState). Do you think it'd make sense to do that in a follow-up? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:44: no_options_text_ = base::ASCIIToUTF16("Adapter turned off."); nit: use IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:76: // |table_view_| is visible. nit: comments that just repeat code aren't worthwhile. ditto elsewhere. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:92: EXPECT_EQ(base::string16(), discovery_state_->text()); nit: EXPECT_TRUE(discovery_state_->text().empty()) https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:369: // |throbber_| is not visible. Can you test the throbber actually being visible in a unit test, or must it be in a browser test? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:371: // |discovery_state_| is disabled and show a label instead of a link. nit: "and shows an empty string" and nix comment below? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:79: IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, InitialState) { It seems like some of these could be unit tests; they don't need a browser. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:80: // |table_view_| is visible. ditto nit here and elsewhere https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:106: EXPECT_EQ(base::string16(), discovery_state_->text()); nit: EXPECT_TRUE(discovery_state_->text().empty()); https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:215: AddAnOptionAndSelectItAndRemoveTheSelectedOption) { How is this usefully different from SelectAnOptionAndRemoveTheSelectedOption? It just adds one option instead of three. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:272: // Since the adapter is turned off, the previously selected option nit: check the option count here and after the adapter is re-enabled? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:291: EXPECT_EQ(base::ASCIIToUTF16("a"), table_model_->GetText(0, 0)); nit: Try to avoid adding tangential checks here and elsewhere in all these new tests; keep each fixture minimal and focused on testing one thing. (This fixture isn't directed toward checking that the options added are really added with the correct strings, but maybe there should be such a fixture if one doesn't already exist?) It's helpful to be thorough by adding many fixtures with unique checks, but it's not helpful to add repeated tangential checks everywhere. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:316: // |discovery_state_| is disabled and show a label instead of a link. nit: shows https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:341: // |discovery_state_| is enabled and show a link. nit: shows
The CQ bit was checked by juncai@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...
https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... File chrome/browser/chooser_controller/mock_chooser_controller.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:40: void MockChooserController::OnAdapterPresenceChanged(AdapterPresence presence) { On 2016/07/22 23:37:04, msw wrote: > It'd be nice if the non-test code used the same enums (AdapterPresence and > DiscoveryState). Do you think it'd make sense to do that in a follow-up? I changed this test code reusing content::BluetoothChooser::AdapterPresence and content::BluetoothChooser::DiscoveryState. Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:44: no_options_text_ = base::ASCIIToUTF16("Adapter turned off."); On 2016/07/22 23:37:04, msw wrote: > nit: use IDS_BLUETOOTH_DEVICE_CHOOSER_ADAPTER_OFF? Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:48: } On 2016/07/22 22:24:53, Reilly Grant wrote: > nit: no braces Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/chooser_cont... chrome/browser/chooser_controller/mock_chooser_controller.cc:57: } On 2016/07/22 22:24:53, Reilly Grant wrote: > nit: no braces Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:76: // |table_view_| is visible. On 2016/07/22 23:37:04, msw wrote: > nit: comments that just repeat code aren't worthwhile. ditto elsewhere. Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:92: EXPECT_EQ(base::string16(), discovery_state_->text()); On 2016/07/22 23:37:04, msw wrote: > nit: EXPECT_TRUE(discovery_state_->text().empty()) Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:369: // |throbber_| is not visible. On 2016/07/22 23:37:04, msw wrote: > Can you test the throbber actually being visible in a unit test, or must it be > in a browser test? I tried testing the throbber in a unit test and it didn't work. Tried running Throbber::Start() at unit test and got DCHECK error at: https://cs.chromium.org/chromium/src/base/threading/thread_task_runner_handle... So I test it in the browser test. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:371: // |discovery_state_| is disabled and show a label instead of a link. On 2016/07/22 23:37:04, msw wrote: > nit: "and shows an empty string" and nix comment below? Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:79: IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, InitialState) { On 2016/07/22 23:37:05, msw wrote: > It seems like some of these could be unit tests; they don't need a browser. Removed some redundant tests that are already in unit tests. Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:80: // |table_view_| is visible. On 2016/07/22 23:37:05, msw wrote: > ditto nit here and elsewhere Removed this line. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:106: EXPECT_EQ(base::string16(), discovery_state_->text()); On 2016/07/22 23:37:04, msw wrote: > nit: EXPECT_TRUE(discovery_state_->text().empty()); Removed this line. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:215: AddAnOptionAndSelectItAndRemoveTheSelectedOption) { On 2016/07/22 23:37:05, msw wrote: > How is this usefully different from SelectAnOptionAndRemoveTheSelectedOption? It > just adds one option instead of three. In this case, since there is only one option added, and after removing it, there is no remaining option, so I think it is worth testing this case? https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:272: // Since the adapter is turned off, the previously selected option On 2016/07/22 23:37:04, msw wrote: > nit: check the option count here and after the adapter is re-enabled? Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:291: EXPECT_EQ(base::ASCIIToUTF16("a"), table_model_->GetText(0, 0)); On 2016/07/22 23:37:04, msw wrote: > nit: Try to avoid adding tangential checks here and elsewhere in all these new > tests; keep each fixture minimal and focused on testing one thing. (This fixture > isn't directed toward checking that the options added are really added with the > correct strings, but maybe there should be such a fixture if one doesn't already > exist?) It's helpful to be thorough by adding many fixtures with unique checks, > but it's not helpful to add repeated tangential checks everywhere. Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:316: // |discovery_state_| is disabled and show a label instead of a link. On 2016/07/22 23:37:04, msw wrote: > nit: shows Done. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:341: // |discovery_state_| is enabled and show a link. On 2016/07/22 23:37:05, msw wrote: > nit: shows Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm, but in general, it'd be nice to add unit tests instead of browser tests, and I'd still prefer to see fewer redundant EXPECT statements and fewer comments that just explain what the next line does when the code itself is straightforward. Feel free to land as-is; these tests are good, and I'm just being a bit nit-picky. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:369: // |throbber_| is not visible. On 2016/07/25 20:14:10, juncai wrote: > On 2016/07/22 23:37:04, msw wrote: > > Can you test the throbber actually being visible in a unit test, or must it be > > in a browser test? > > I tried testing the throbber in a unit test and it didn't work. Tried running > Throbber::Start() at unit test and got DCHECK error at: > https://cs.chromium.org/chromium/src/base/threading/thread_task_runner_handle... > > So I test it in the browser test. Can you try constructing a base::TestSimpleTaskRunner and base::ThreadTaskRunnerHandle pair in the unit test (or similar)? Avoiding browser tests is nice, when possible. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/ext... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:215: AddAnOptionAndSelectItAndRemoveTheSelectedOption) { On 2016/07/25 20:14:10, juncai wrote: > On 2016/07/22 23:37:05, msw wrote: > > How is this usefully different from SelectAnOptionAndRemoveTheSelectedOption? > It > > just adds one option instead of three. > > In this case, since there is only one option added, and after removing it, there > is no remaining option, so I think it is worth testing this case? I suppose it's okay. https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:26: class ChooserDialogViewTest : public ExtensionBrowserTest { It'd be nice to make all these test fixtures unit tests instead of browser tests (seems possible); but I won't block on that.
The CQ bit was checked by juncai@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 checked by juncai@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...
Thanks for all the comments! Removed some redundant comments and EXPECT statements. Moved throbber related test code from browser test to unit test. https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... File chrome/browser/ui/views/chooser_content_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/1/chrome/browser/ui/views/cho... chrome/browser/ui/views/chooser_content_view_unittest.cc:369: // |throbber_| is not visible. On 2016/07/26 00:18:56, msw wrote: > On 2016/07/25 20:14:10, juncai wrote: > > On 2016/07/22 23:37:04, msw wrote: > > > Can you test the throbber actually being visible in a unit test, or must it > be > > > in a browser test? > > > > I tried testing the throbber in a unit test and it didn't work. Tried running > > Throbber::Start() at unit test and got DCHECK error at: > > > https://cs.chromium.org/chromium/src/base/threading/thread_task_runner_handle... > > > > So I test it in the browser test. > > Can you try constructing a base::TestSimpleTaskRunner and > base::ThreadTaskRunnerHandle pair in the unit test (or similar)? Avoiding > browser tests is nice, when possible. Thanks! Added base::TestSimpleTaskRunner and base::ThreadTaskRunnerHandle pair in the unit test and the throbber works. So moved throbber related test code from browser test to unit test. The only remaining part at browser test is to test the ok and cancel buttons. I guess they have to be tested in the browser test? https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:26: class ChooserDialogViewTest : public ExtensionBrowserTest { On 2016/07/26 00:18:56, msw wrote: > It'd be nice to make all these test fixtures unit tests instead of browser tests > (seems possible); but I won't block on that. Now the only remaining part at browser test is to test the ok and cancel buttons. I guess they need to be tested in the browser test?
still lgtm https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:26: class ChooserDialogViewTest : public ExtensionBrowserTest { On 2016/07/26 23:30:01, juncai wrote: > On 2016/07/26 00:18:56, msw wrote: > > It'd be nice to make all these test fixtures unit tests instead of browser > tests > > (seems possible); but I won't block on that. > > Now the only remaining part at browser test is to test the ok and cancel > buttons. I guess they need to be tested in the browser test? Afaict, the only change you'd need to make to move these to unit tests would be passing in ViewsTestBase::GetContext() for the context (or some dummy parent window), instead of the browser's native window for CreateDialogWidget. There isn't any browser-specific behavior, afaict. That said, this is already an improvement; feel free to land as-is and optionally follow up separately, but I'll also gladly re-review if you try to move the remaining tests in this CL.
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 juncai@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...
https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (right): https://codereview.chromium.org/2171103002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:26: class ChooserDialogViewTest : public ExtensionBrowserTest { On 2016/07/26 23:47:31, msw wrote: > On 2016/07/26 23:30:01, juncai wrote: > > On 2016/07/26 00:18:56, msw wrote: > > > It'd be nice to make all these test fixtures unit tests instead of browser > > tests > > > (seems possible); but I won't block on that. > > > > Now the only remaining part at browser test is to test the ok and cancel > > buttons. I guess they need to be tested in the browser test? > > Afaict, the only change you'd need to make to move these to unit tests would be > passing in ViewsTestBase::GetContext() for the context (or some dummy parent > window), instead of the browser's native window for CreateDialogWidget. There > isn't any browser-specific behavior, afaict. That said, this is already an > improvement; feel free to land as-is and optionally follow up separately, but > I'll also gladly re-review if you try to move the remaining tests in this CL. Changed chooser_dialog_view_browsertest.cc to be chooser_dialog_view_unittest.cc. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Very awesome! lgtm! https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (left): https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:1: // Copyright 2016 The Chromium Authors. All rights reserved. nit: this file is marked as modified, not deleted in Rietveld, make sure the file is actually removed in your "git status" (sometimes that just happens, but worth checking). https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:13: #include "testing/gtest/include/gtest/gtest.h" optional nit: not needed (included via views_test_base.h)
The CQ bit was checked by juncai@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.
https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc (left): https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc:1: // Copyright 2016 The Chromium Authors. All rights reserved. On 2016/07/27 20:25:46, msw wrote: > nit: this file is marked as modified, not deleted in Rietveld, make sure the > file is actually removed in your "git status" (sometimes that just happens, but > worth checking). I used "git mv" to rename it to chooser_dialog_view_unittest.cc. I checked and the file doesn't exist in the directory. https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... File chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc (right): https://codereview.chromium.org/2171103002/diff/80001/chrome/browser/ui/views... chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:13: #include "testing/gtest/include/gtest/gtest.h" On 2016/07/27 20:25:46, msw wrote: > optional nit: not needed (included via views_test_base.h) Done.
The CQ bit was checked by juncai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from reillyg@chromium.org, msw@chromium.org Link to the patchset: https://codereview.chromium.org/2171103002/#ps100001 (title: "address comments")
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 #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Add test for WebBluetooth chooser spinner, status text and rescan button This is part 5 of matching WebBluetooth chooser views to mocks. This CL adds test code for spinner, status text and rescan button that was added in previous patches. BUG=583452 ========== to ========== Add test for WebBluetooth chooser spinner, status text and rescan button This is part 5 of matching WebBluetooth chooser views to mocks. This CL adds test code for spinner, status text and rescan button that was added in previous patches. BUG=583452 Committed: https://crrev.com/68973c4f29e3cfdee4edaca1768c0db9635190b7 Cr-Commit-Position: refs/heads/master@{#408247} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/68973c4f29e3cfdee4edaca1768c0db9635190b7 Cr-Commit-Position: refs/heads/master@{#408247} |
