Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2564173002:
MANUAL tests of GDI capturers (Closed)
DescriptionMANUAL tests of GDI capturers
ScreenCapturerWinGdi randomly returns black frames in test environment. The root
cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to
execute in test environment, but unblock other developers. We can eventually get
a failure ratio and more samples for debugging.
BUG=webrtc:6666, webrtc:6843
Committed: https://crrev.com/8d1649d71b74d511c508b0b3d41faf8c1a075c70
Cr-Commit-Position: refs/heads/master@{#15518}
Patch Set 1 #
Total comments: 3
Messages
Total messages: 22 (14 generated)
The CQ bit was checked by zijiehe@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.webrtc.org/...
The CQ bit was checked by zijiehe@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.webrtc.org/...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. So we can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 ========== to ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. We can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 ==========
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
Normally we mark tests as DISABLED_ to disable them. LGTM otherwise. https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/screen_capturer_integration_test.cc (right): https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/screen_capturer_integration_test.cc:290: #define MAYBE_CaptureUpdatedRegion MANUAL_CaptureUpdatedRegion s/DISABLE_/MANUAL_/
https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/screen_capturer_integration_test.cc (right): https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/screen_capturer_integration_test.cc:290: #define MAYBE_CaptureUpdatedRegion MANUAL_CaptureUpdatedRegion On 2016/12/09 23:18:01, Sergey Ulanov wrote: > s/DISABLE_/MANUAL_/ There is a mail thread between Henrik and me, we will use the MANUAL_ prefix to execute these tests in waterfall without impacting other developers.
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481327901581950,
"parent_rev": "66f931cbad60b7d2e8d0a848e356276061e5bddb", "commit_rev":
"4c19aa7a782c4b2982c4806cda8e23e798a58e65"}
Message was sent while issue was closed.
Description was changed from ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. We can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 ========== to ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. We can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 Review-Url: https://codereview.webrtc.org/2564173002 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. We can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 Review-Url: https://codereview.webrtc.org/2564173002 ========== to ========== MANUAL tests of GDI capturers ScreenCapturerWinGdi randomly returns black frames in test environment. The root cause is still unknown, so change ScreenCapturerWinGdi tests into MANUAL mode to execute in test environment, but unblock other developers. We can eventually get a failure ratio and more samples for debugging. BUG=webrtc:6666, webrtc:6843 Committed: https://crrev.com/8d1649d71b74d511c508b0b3d41faf8c1a075c70 Cr-Commit-Position: refs/heads/master@{#15518} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/8d1649d71b74d511c508b0b3d41faf8c1a075c70 Cr-Commit-Position: refs/heads/master@{#15518}
Message was sent while issue was closed.
kjellander@webrtc.org changed reviewers: + kjellander@webrtc.org
Message was sent while issue was closed.
I was wrong about MANUAL_, it only works in Chrome. See my comment. https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/screen_capturer_integration_test.cc (right): https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/screen_capturer_integration_test.cc:290: #define MAYBE_CaptureUpdatedRegion MANUAL_CaptureUpdatedRegion On 2016/12/09 23:33:19, Hzj_jie wrote: > On 2016/12/09 23:18:01, Sergey Ulanov wrote: > > s/DISABLE_/MANUAL_/ > > There is a mail thread between Henrik and me, we will use the MANUAL_ prefix to > execute these tests in waterfall without impacting other developers. It turns out I misunderstood that MANUAL_ was a part of the supported gtest prefixes. It's something that Chromium implemented only for their test launcher: https://cs.chromium.org/chromium/src/content/public/test/test_launcher.cc?rcl... Luckily, we can use DISABLED_ for the same purpose, since there's the --gtest_also_run_disabled_tests flag. I submitted https://codereview.webrtc.org/2568013002 to change this.
Message was sent while issue was closed.
On 2016/12/12 07:30:31, kjellander_webrtc wrote: > I was wrong about MANUAL_, it only works in Chrome. See my comment. > > https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... > File webrtc/modules/desktop_capture/screen_capturer_integration_test.cc (right): > > https://codereview.webrtc.org/2564173002/diff/20001/webrtc/modules/desktop_ca... > webrtc/modules/desktop_capture/screen_capturer_integration_test.cc:290: #define > MAYBE_CaptureUpdatedRegion MANUAL_CaptureUpdatedRegion > On 2016/12/09 23:33:19, Hzj_jie wrote: > > On 2016/12/09 23:18:01, Sergey Ulanov wrote: > > > s/DISABLE_/MANUAL_/ > > > > There is a mail thread between Henrik and me, we will use the MANUAL_ prefix > to > > execute these tests in waterfall without impacting other developers. > > It turns out I misunderstood that MANUAL_ was a part of the supported gtest > prefixes. It's something that Chromium implemented only for their test launcher: > https://cs.chromium.org/chromium/src/content/public/test/test_launcher.cc?rcl... > > Luckily, we can use DISABLED_ for the same purpose, since there's the > --gtest_also_run_disabled_tests flag. I submitted > https://codereview.webrtc.org/2568013002 to change this. I thought you were trying to implement the same mechanism in WebRTC. I am OK with your change as long as we have a way to do so. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
