Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2359243002:
VideoCaptureDeviceTest: improve error logging (Closed)
|
Created:
4 years, 3 months ago by mcasas Modified:
4 years, 3 months ago Reviewers:
chfremer CC:
chromium-reviews, posciak+watch_chromium.org, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, miu+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionVideoCaptureDeviceTest: improve error logging
Associated bug's #3 is a revert and got me thinking that we don't log
the error received in the VCDTest's MockVideoCaptureDeviceClient's, so
this CL adds a small ON_CALL for it. The difference is that, e.g. in
the OpenInvalidDevice test case, we get an extra line with the error
origin, message and the OS/System error message, if any.
[ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice
[...]
[58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory
BUG=647352
Committed: https://crrev.com/f5665dc8fa767fc1493e0549f4bd813e26d74dff
Cr-Commit-Position: refs/heads/master@{#420423}
Patch Set 1 : #
Total comments: 2
Patch Set 2 : chfremer@ comment #
Total comments: 1
Messages
Total messages: 14 (7 generated)
Patchset #1 (id:1) has been deleted
Description was changed from ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with "On Error message:", the VCD log and then the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [57913:1295:0922/101626:1086711639009777:ERROR:video_capture_device_unittest.cc(83)] OnError message: Could not open capture device.: No such file or directory BUG=647352 ========== to ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with "On Error message:", the VCD log and then the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ==========
Description was changed from ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with "On Error message:", the VCD log and then the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ========== to ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with the error origin, message and the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ==========
mcasas@chromium.org changed reviewers: + chfremer@chromium.org
chfremer@ PTAL
https://codereview.chromium.org/2359243002/diff/20001/media/capture/video/vid... File media/capture/video/video_capture_device_unittest.cc (right): https://codereview.chromium.org/2359243002/diff/20001/media/capture/video/vid... media/capture/video/video_capture_device_unittest.cc:102: .WillByDefault(WithArgs<0, 1>(Invoke(DumpError))); Is the WithArgs<0, 1> really needed? It seems the parameters to OnError() already match the parameters of DumpError.
https://codereview.chromium.org/2359243002/diff/20001/media/capture/video/vid... File media/capture/video/video_capture_device_unittest.cc (right): https://codereview.chromium.org/2359243002/diff/20001/media/capture/video/vid... media/capture/video/video_capture_device_unittest.cc:102: .WillByDefault(WithArgs<0, 1>(Invoke(DumpError))); On 2016/09/22 17:44:18, chfremer wrote: > Is the WithArgs<0, 1> really needed? It seems the parameters to OnError() > already match the parameters of DumpError. Ooops, isn't needed indeed, it's a leftover from an intermediate point. Removed.
lgtm https://codereview.chromium.org/2359243002/diff/40001/media/capture/video/vid... File media/capture/video/video_capture_device_unittest.cc (right): https://codereview.chromium.org/2359243002/diff/40001/media/capture/video/vid... media/capture/video/video_capture_device_unittest.cc:100: ON_CALL(*this, OnError(_, _)).WillByDefault(Invoke(DumpError)); Sgtm. Alternatively, we could use a Lambda expression here, but other than saving the identifier "DumpError" it does not make a difference, and I think the way it is now may be easier to read.
The CQ bit was checked by mcasas@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 ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with the error origin, message and the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ========== to ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with the error origin, message and the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with the error origin, message and the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 ========== to ========== VideoCaptureDeviceTest: improve error logging Associated bug's #3 is a revert and got me thinking that we don't log the error received in the VCDTest's MockVideoCaptureDeviceClient's, so this CL adds a small ON_CALL for it. The difference is that, e.g. in the OpenInvalidDevice test case, we get an extra line with the error origin, message and the OS/System error message, if any. [ RUN ] VideoCaptureDeviceTest.OpenInvalidDevice [...] [58186:1295:0922/102314:1087119696054855:ERROR:video_capture_device_unittest.cc(84)] AllocateAndStart@../../media/capture/video/mac/video_capture_device_mac.mm:326 Could not open capture device.: No such file or directory BUG=647352 Committed: https://crrev.com/f5665dc8fa767fc1493e0549f4bd813e26d74dff Cr-Commit-Position: refs/heads/master@{#420423} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f5665dc8fa767fc1493e0549f4bd813e26d74dff Cr-Commit-Position: refs/heads/master@{#420423} |
