Add more logging in ScreenCapturerIntegrationTest

webrtc/modules/desktop_capture/screen_capturer_integration_test.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <string.h>
 
 #include <algorithm>
 #include <initializer_list>
 #include <memory>
 #include <utility>
 
+// TODO(zijiehe): Remove once flaky has been resolved.
+#include "webrtc/base/base64.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/modules/desktop_capture/desktop_capturer.h"
 #include "webrtc/modules/desktop_capture/desktop_capture_options.h"
 #include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/desktop_region.h"
 #include "webrtc/modules/desktop_capture/mock_desktop_capturer_callback.h"
 #include "webrtc/modules/desktop_capture/rgba_color.h"
 #include "webrtc/modules/desktop_capture/screen_drawer.h"
@@ skipping 81 matching lines @@
     // by |c|) its primary colors is |i|, and the other two are 0x7f. So we
     // won't draw a black or white rectangle.
     for (int c = 0; c < 3; c++) {
       // A fixed size rectangle.
       for (int i = 0; i < kTestArea - kRectSize; i += 16) {
         DesktopRect rect = DesktopRect::MakeXYWH(i, i, kRectSize, kRectSize);
         rect.Translate(drawer->DrawableRegion().top_left());
         RgbaColor color((c == 0 ? (i & 0xff) : 0x7f),
                         (c == 1 ? (i & 0xff) : 0x7f),
                         (c == 2 ? (i & 0xff) : 0x7f));
-        TestCaptureOneFrame(capturers, drawer.get(), rect, color);
+        // Fail fast.
+        ASSERT_NO_FATAL_FAILURE(
+            TestCaptureOneFrame(capturers, drawer.get(), rect, color));
       }
 
       // A variable-size rectangle.
       for (int i = 0; i < kTestArea - kRectSize; i += 16) {
         DesktopRect rect = DesktopRect::MakeXYWH(kRectSize, kRectSize, i, i);
         rect.Translate(drawer->DrawableRegion().top_left());
         RgbaColor color((c == 0 ? (i & 0xff) : 0x7f),
                         (c == 1 ? (i & 0xff) : 0x7f),
                         (c == 2 ? (i & 0xff) : 0x7f));
-        TestCaptureOneFrame(capturers, drawer.get(), rect, color);
+        // Fail fast.
+        ASSERT_NO_FATAL_FAILURE(
+            TestCaptureOneFrame(capturers, drawer.get(), rect, color));
       }
     }
   }
 
   void TestCaptureUpdatedRegion() {
     TestCaptureUpdatedRegion({capturer_.get()});
   }
 
 #if defined(WEBRTC_WIN)
   // Enable allow_directx_capturer in DesktopCaptureOptions, but let
@@ skipping 36 matching lines @@
                            RgbaColor color) {
     const int wait_capture_round = 600;
     drawer->Clear();
     size_t succeeded_capturers = 0;
     for (int i = 0; i < wait_capture_round; i++) {
       drawer->DrawRectangle(rect, color);
       drawer->WaitForPendingDraws();
       for (size_t j = 0; j < capturers.size(); j++) {
         if (capturers[j] == nullptr) {
           // DesktopCapturer should return an empty updated_region() if no
-          // update detected. So we won't test it again if it has captured
-          // the rectangle we drew.
+          // update detected. So we won't test it again if it has captured the
+          // rectangle we drew.
           continue;
         }
         std::unique_ptr<DesktopFrame> frame = CaptureFrame(capturers[j]);
         if (!frame) {
-          // CaptureFrame() has triggered an assertion failure already, we
-          // only need to return here.
+          // CaptureFrame() has triggered an assertion failure already, we only
+          // need to return here.
           return;
         }
 
         if (ArePixelsColoredBy(
             *frame, rect, color, drawer->MayDrawIncompleteShapes())) {
           capturers[j] = nullptr;
           succeeded_capturers++;
         }
+        // The following else if statement is for debugging purpose only, which
+        // should be removed after flaky of ScreenCapturerIntegrationTest has
+        // been resolved.
+        else if (i == wait_capture_round - 1) {
+          std::string result;
+          rtc::Base64::EncodeFromArray(frame->data(),
+                                       frame->size().height() * frame->stride(),
+                                       &result);
+          LOG(LS_INFO) << frame->size().width() << " x "
+                       << frame->size().height();
+          // Split the entire string (can be over 4M) into several lines to

[kjellander_webrtc, 2016/11/18 08:47:23]

How large can this be? The swarming trybots have a limit of 16MB of logs, so
going above that will cause tests to fail (and I'm not sure we'll even get these
lines logged).

There's a special directory we can pass to the script where output files that
shall be archived can be written.
Ideally we'll make use of that and write the images in their original format to
that dir instead. For that we need to have a flag that makes the directory
available for the test.
I filed https://bugs.chromium.org/p/webrtc/issues/detail?id=6732 for
implementing that, so you might want to wait for that (or look at implementing
it yourself).

That said, since this is only written upon failure, we could try this approach
first either way.

[Hzj_jie, 2016/11/18 22:07:25]

On our trybots, it should be 4M. Since the test will fail immediately after one
logging, it should be OK.

I do recall LOG(LS_INFO) will be ignored by trybot, so I used std::out in the
first iteration. And also, printf / std::out are widely used in other tests,
such as https://goo.gl/JNYVah and https://goo.gl/VFVaAj. So I would prefer to
revert the change, and use std::out.

I am happy with the change, and I believe we can even go further. For example,
in ScreenCapturerIntegrationTest, a compression is really welcome.

But we can do two things simultaneous, I would prefer to submit this change and
a following change to enable ScreenCapturerIntegrationTests again, and see if we
can get a valuable screenshot. Meanwhile, we can work on the tools for test
cases to store large data.

Sounds good?

[kjellander_webrtc, 2016/11/21 06:58:12]

The running test case fails, yes, but the remaining test methods are still
executed, which might
use the same debug logging upon additional failures, since
TestCaptureUpdatedRegion is used by multiple test cases.
Since it's only printed on failures, it's OK. Let's see how it goes.

+          // avoid browser from stucking.
+          static const size_t kLineLength = 32768;
+          const char* result_end = result.c_str() + result.length();
+          for (const char* it = result.c_str();
+               it < result_end;
+               it += kLineLength) {
+            const size_t max_length = result_end - it;
+            LOG(LS_INFO) << std::string(it, std::min(kLineLength, max_length));
+          }
+          ASSERT_TRUE(false) << "ScreenCapturerIntegrationTest is flaky. "

[kjellander_webrtc, 2016/11/18 08:47:23]

Maybe it's a bit too strong of a claim to say that it's flaky when it fails. I
mean it could also fail due to a code change, right? I'd suggest rephrasing to
something that tells it might be flaky (since it has been in the past).

[Hzj_jie, 2016/11/18 22:07:25]

Updated.

+                                "Please disable *all* tests in "
+                                "screen_capturer_integration_test.cc, and "
+                                "kindly FYI the broken link to "
+                                "zijiehe@chromium.org";
+        }
       }
 
       if (succeeded_capturers == capturers.size()) {
         break;
       }
     }
 
     ASSERT_EQ(succeeded_capturers, capturers.size());
   }
 
@@ skipping 71 matching lines @@
        DISABLED_MaybeCaptureUpdatedRegionWithDirectxCapturer) {
   // Even DirectX capturer is not supported in current system, we should be able
   // to select a usable capturer.
   MaybeCreateDirectxCapturer();
   TestCaptureUpdatedRegion();
 }
 
 #endif  // defined(WEBRTC_WIN)
 
 }  // namespace webrtc