Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2333983004:
Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. (Closed)
DescriptionRefactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver.
This change is based on
https://codereview.chromium.org/2292473002/
which was once merged but reverted by
https://codereview.chromium.org/2340433002/
BUG=641943
Committed: https://crrev.com/a07c4fb1ece6ee6c6a2170825208b1f873012a0a
Cr-Commit-Position: refs/heads/master@{#418478}
Patch Set 1 : copy of 2292473002. #Patch Set 2 : Delete the observer object in TearDown. #
Total comments: 2
Patch Set 3 : wrap with unique_ptr<> #Patch Set 4 : Sync to head. #
Total comments: 1
Messages
Total messages: 33 (21 generated)
The CQ bit was checked by yamaguchi@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.
Description was changed from ========== Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. disk_mount_manager_unittest.cc will be rewritten using this class instead of gmock. BUG=641943 ========== to ========== Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. disk_mount_manager_unittest.cc will be rewritten using this class instead of gmock. This change is based on https://codereview.chromium.org/2292473002/ which was reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ==========
Description was changed from ========== Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. disk_mount_manager_unittest.cc will be rewritten using this class instead of gmock. This change is based on https://codereview.chromium.org/2292473002/ which was reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ========== to ========== Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. disk_mount_manager_unittest.cc will be rewritten using this class instead of gmock. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ==========
yamaguchi@chromium.org changed reviewers: + satorux@google.com
yamaguchi@chromium.org changed reviewers: + satorux@chromium.org - satorux@google.com
I forgot to notice with the original patch, but the patch description is outdated. Please update!
https://codereview.chromium.org/2333983004/diff/20001/chromeos/disks/disk_mou... File chromeos/disks/disk_mount_manager_unittest.cc (right): https://codereview.chromium.org/2333983004/diff/20001/chromeos/disks/disk_mou... chromeos/disks/disk_mount_manager_unittest.cc:435: delete(observer_); can we wrap observer_ std:unique_ptr<> instead?
Description was changed from ========== Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests. disk_mount_manager_unittest.cc will be rewritten using this class instead of gmock. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ========== to ========== Refactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ==========
The CQ bit was checked by yamaguchi@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: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) 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 yamaguchi@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...
Also updated the description. ptal. https://codereview.chromium.org/2333983004/diff/20001/chromeos/disks/disk_mou... File chromeos/disks/disk_mount_manager_unittest.cc (right): https://codereview.chromium.org/2333983004/diff/20001/chromeos/disks/disk_mou... chromeos/disks/disk_mount_manager_unittest.cc:435: delete(observer_); On 2016/09/14 01:41:59, satorux1 wrote: > can we wrap observer_ std:unique_ptr<> instead? Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by satorux@chromium.org
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.
Description was changed from ========== Refactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ========== to ========== Refactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Refactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 ========== to ========== Refactor disk_mount_manager_unittest by replacing gmock with a dedicated mock class of DiskMountManagerObserver. This change is based on https://codereview.chromium.org/2292473002/ which was once merged but reverted by https://codereview.chromium.org/2340433002/ BUG=641943 Committed: https://crrev.com/a07c4fb1ece6ee6c6a2170825208b1f873012a0a Cr-Commit-Position: refs/heads/master@{#418478} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/a07c4fb1ece6ee6c6a2170825208b1f873012a0a Cr-Commit-Position: refs/heads/master@{#418478}
Message was sent while issue was closed.
FYI: this CL is causing test failures on waterfall. https://findit-for-me.appspot.com/waterfall/build-failure?url=https%3A%2F%2Fb...
Message was sent while issue was closed.
On 2016/09/14 08:08:21, stgao (slow) wrote: > FYI: this CL is causing test failures on waterfall. > > https://findit-for-me.appspot.com/waterfall/build-failure?url=https%3A%2F%2Fb... Thanks, I will revert and fix it.
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2343593002/ by yamaguchi@chromium.org. The reason for reverting is: Asan failure.
Message was sent while issue was closed.
fukino@chromium.org changed reviewers: + fukino@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2333983004/diff/60001/chromeos/disks/disk_mou... File chromeos/disks/disk_mount_manager_unittest.cc (right): https://codereview.chromium.org/2333983004/diff/60001/chromeos/disks/disk_mou... chromeos/disks/disk_mount_manager_unittest.cc:165: struct ObserverEvent { Virtual destructor should be required in this base class to delete the instance of derived class. |
