Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2938023002:
[PresentationService] Improve PresentationServiceImplTest. (Closed)
Description[PresentationService] Improve PresentationServiceImplTest.
- Remove PresentationServicePtr - the unit tests should just invoke
the PresentationServiceImpl directly for simplicity. This also allows
us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and
replace with RunLoop::RunUntilIdle.
- Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo
PresentationService overrides to public.
- Remove other unused functions in the test.
BUG=699225
Review-Url: https://codereview.chromium.org/2938023002
Cr-Commit-Position: refs/heads/master@{#479748}
Committed: https://chromium.googlesource.com/chromium/src/+/fafb67d05a69c02a39a451f36a169bd76f10f453
Patch Set 1 : . #
Total comments: 2
Patch Set 2 : Addressed comments #Patch Set 3 : Rebase #
Messages
Total messages: 25 (18 generated)
Description was changed from ========== remove messagign more fixes ok except for messaing tests maybe BUG= ========== to ========== [PresentationService] Improve PresentationServiceImplTest. - Remove PresentationServicePtr - the unit tests should just invoke the PresentationServiceImpl directly for simplicity. This also allows us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and replace with RunLoop::RunUntilIdle. - Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo PresentationService overrides to public. - Remove other unused functions in the test. BUG=699225 ==========
Description was changed from ========== [PresentationService] Improve PresentationServiceImplTest. - Remove PresentationServicePtr - the unit tests should just invoke the PresentationServiceImpl directly for simplicity. This also allows us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and replace with RunLoop::RunUntilIdle. - Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo PresentationService overrides to public. - Remove other unused functions in the test. BUG=699225 ========== to ========== [PresentationService] Improve PresentationServiceImplTest. - Remove PresentationServicePtr - the unit tests should just invoke the PresentationServiceImpl directly for simplicity. This also allows us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and replace with RunLoop::RunUntilIdle. - Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo PresentationService overrides to public. - Remove other unused functions in the test. BUG=699225 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
imcheng@chromium.org changed reviewers: + zhaobin@chromium.org
Bin: PTAL, thanks!
https://codereview.chromium.org/2938023002/diff/40001/content/browser/present... File content/browser/presentation/presentation_service_impl_unittest.cc (right): https://codereview.chromium.org/2938023002/diff/40001/content/browser/present... content/browser/presentation/presentation_service_impl_unittest.cc:307: void ExpectNewPresentationCallbackSuccess( Would it be simpler if we use MockCallback instead of ExpectNewPresentationCallbackSuccess and ExpectNewPresentationCallbackError?
https://codereview.chromium.org/2938023002/diff/40001/content/browser/present... File content/browser/presentation/presentation_service_impl_unittest.cc (right): https://codereview.chromium.org/2938023002/diff/40001/content/browser/present... content/browser/presentation/presentation_service_impl_unittest.cc:307: void ExpectNewPresentationCallbackSuccess( On 2017/06/15 02:40:25, zhaobin wrote: > Would it be simpler if we use MockCallback instead of > ExpectNewPresentationCallbackSuccess and ExpectNewPresentationCallbackError? It would certainly help us get rid of the 2 helper methods and the counters. However, I had to build custom matchers as I didn't find ones that work with base::Optional.
The CQ bit was checked by imcheng@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: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by imcheng@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.
lgtm
mfoltz@chromium.org changed reviewers: + mfoltz@chromium.org
lgtm Thanks for doing this, much nicer :-)
The CQ bit was checked by imcheng@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1497547553510870,
"parent_rev": "2ec4013ac56a7f30db385949269adc21ae980bf3", "commit_rev":
"20a7aa8ca168dd467e1b6763fe822df059e0d536"}
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1497547553510870,
"parent_rev": "35266f16e54a84c33dc4647b5d16a36edd54ba10", "commit_rev":
"fafb67d05a69c02a39a451f36a169bd76f10f453"}
Message was sent while issue was closed.
Description was changed from ========== [PresentationService] Improve PresentationServiceImplTest. - Remove PresentationServicePtr - the unit tests should just invoke the PresentationServiceImpl directly for simplicity. This also allows us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and replace with RunLoop::RunUntilIdle. - Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo PresentationService overrides to public. - Remove other unused functions in the test. BUG=699225 ========== to ========== [PresentationService] Improve PresentationServiceImplTest. - Remove PresentationServicePtr - the unit tests should just invoke the PresentationServiceImpl directly for simplicity. This also allows us to remove the usage of RunLoop::QuitClosure and RunLoop::Run and replace with RunLoop::RunUntilIdle. - Remove a bunch of FRIEND_CLASS_TEST_PREFIX and move the Mojo PresentationService overrides to public. - Remove other unused functions in the test. BUG=699225 Review-Url: https://codereview.chromium.org/2938023002 Cr-Commit-Position: refs/heads/master@{#479748} Committed: https://chromium.googlesource.com/chromium/src/+/fafb67d05a69c02a39a451f36a16... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/chromium/src/+/fafb67d05a69c02a39a451f36a16... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
