Add new API function: feedbackPrivate.readLogSource

chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc

Index: chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc
diff --git a/chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc b/chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc
index de42a7ec285ea2d6665e4efe2a1d94c57cf50062..8ce2db1bce0407b3d9d06deb9e8e9697e0e4be33 100644
--- a/chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc
+++ b/chrome/browser/extensions/api/feedback_private/feedback_private_apitest.cc
@@ -3,12 +3,69 @@
 // found in the LICENSE file.
 
 #include "base/message_loop/message_loop.h"
+#include "base/task_scheduler/post_task.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/api/feedback_private/feedback_private_api.h"
+#include "chrome/browser/extensions/api/feedback_private/single_log_source_factory.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 
 namespace extensions {
 
+namespace {
+
+using system_logs::SingleLogSource;
+using system_logs::SystemLogsResponse;
+using SupportedSource = system_logs::SingleLogSource::SupportedSource;
+
+// A dummy SingleLogSource that does not require real system logs to be
+// available during testing.
+class TestSingleLogSource : public SingleLogSource {
+ public:
+  explicit TestSingleLogSource(SupportedSource type)
+      : SingleLogSource(type), call_count_(0) {}
+
+  ~TestSingleLogSource() override {}
+
+  // Fetch() will return a single different string each time, in the following
+  // sequence: "a", "bb", "ccc", until a string of 26 z's. Will never return an
+  // empty result.
+  void Fetch(const system_logs::SysLogsSourceCallback& callback) override {
+    int count_modulus = call_count_ % kNumCharsToIterate;
+    std::string result(count_modulus + 1, kInitialChar + count_modulus);
+    CHECK_GT(result.size(), 0U);
+    ++call_count_;
+
+    SystemLogsResponse* result_map = new SystemLogsResponse;
+    result_map->emplace("", result);
+
+    // Do not directly pass the result to the callback, because that's not how
+    // log sources actually work. Instead, simulate the asynchronous operation
+    // of a SingleLogSource by invoking the callback after a delay.
+    base::PostDelayedTaskWithTraits(
+        FROM_HERE, base::TaskPriority::BACKGROUND,
+        base::Bind(callback, base::Owned(result_map)),
+        base::TimeDelta::FromMilliseconds(100));

[tbarzic, 2017/05/22 23:02:55]

I think you should use 0 delay here, that should be enough to simulate async
call (and avoid holding up the test without a reason).

If you want to test that the callback timing respects the timeout value, you
should use unittests, where you can mock message loop time.

[Simon Que, 2017/05/23 20:01:19]

Done.

+  }
+
+  // Instantiates a new instance of this class. Does not retain ownership. Used
+  // to create a Callback that can be used to override the default behavior of
+  // SingleLogSourceFactory.
+  static SingleLogSource* Create(SupportedSource type) {
+    return new TestSingleLogSource(type);
+  }
+
+ private:
+  // Iterate over the whole lowercase alphabet, starting from 'a'.
+  const int kNumCharsToIterate = 26;
+  const char kInitialChar = 'a';
+
+  // Keep track of how many times Fetch() has been called, in order to determine
+  // its behavior each time.
+  int call_count_;

[tbarzic, 2017/05/22 23:02:55]

add DISALLOW_COPY_AND_ASSIGN

[Simon Que, 2017/05/23 20:01:19]

Done.

+};
+
+}  // namespace
+
 class FeedbackApiTest: public ExtensionApiTest {
  public:
   FeedbackApiTest() {}
@@ -19,4 +76,37 @@ IN_PROC_BROWSER_TEST_F(FeedbackApiTest, Basic) {
   EXPECT_TRUE(RunExtensionTest("feedback_private/basic")) << message_;
 }
 
+#if defined(OS_CHROMEOS)
+IN_PROC_BROWSER_TEST_F(FeedbackApiTest, ReadLogSource) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(0));
+  FeedbackPrivateReadLogSourceFunction::SetRateLimitingTimeoutForTesting(
+      &timeout);
+  SingleLogSourceFactory::CreateCallback callback =
+      base::Bind(&TestSingleLogSource::Create);
+  SingleLogSourceFactory::InitForTesting(&callback);
+
+  EXPECT_TRUE(RunExtensionTest("feedback_private/read_log_source")) << message_;
+
+  SingleLogSourceFactory::InitForTesting(nullptr);
+  FeedbackPrivateReadLogSourceFunction::SetRateLimitingTimeoutForTesting(
+      nullptr);
+}
+
+IN_PROC_BROWSER_TEST_F(FeedbackApiTest, ReadLogSourceRateLimited) {
+  const base::TimeDelta timeout(base::TimeDelta::FromMilliseconds(2000));

[tbarzic, 2017/05/22 23:02:55]

we should not hold up tests for 2 seconds :/

[Simon Que, 2017/05/23 20:01:19]

Agreed, but the alarm API used by the test app doesn't seem to fire more than
once per second. Maybe we should switch to C++-based API unittest?

+  FeedbackPrivateReadLogSourceFunction::SetRateLimitingTimeoutForTesting(
+      &timeout);
+  SingleLogSourceFactory::CreateCallback callback =
+      base::Bind(&TestSingleLogSource::Create);

[tbarzic, 2017/05/22 23:02:55]

can you do this in FeedbackApiTest::SetUp?

[Simon Que, 2017/05/23 20:01:19]

Done. Not sure if I'm doing it right though.

+  SingleLogSourceFactory::InitForTesting(&callback);
+
+  EXPECT_TRUE(RunExtensionTest("feedback_private/read_log_source_rate_limited"))
+      << message_;
+
+  SingleLogSourceFactory::InitForTesting(nullptr);
+  FeedbackPrivateReadLogSourceFunction::SetRateLimitingTimeoutForTesting(
+      nullptr);
+}
+#endif  // defined(OS_CHROMEOS)
+
 } // namespace extensions