Blimp: Add BlobChannelReceiver and bindings interface.

blimp/net/blob_channel/blob_channel_receiver_unittest.cc

Index: blimp/net/blob_channel/blob_channel_receiver_unittest.cc
diff --git a/blimp/net/blob_channel/blob_channel_receiver_unittest.cc b/blimp/net/blob_channel/blob_channel_receiver_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..baed9dede9cb3081ede426ab0ce50a45d0e51ff4
--- /dev/null
+++ b/blimp/net/blob_channel/blob_channel_receiver_unittest.cc
@@ -0,0 +1,86 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <algorithm>
+#include <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
+#include "blimp/common/blob_cache/mock_blob_cache.h"
+#include "blimp/net/blob_channel/blob_channel_receiver.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blimp {
+namespace {
+
+using testing::_;
+using testing::Return;
+
+const char kBlobId1[] = "blob-1";
+const char kBlobPayload1[] = "blob-1-payload";
+
+// Helper function for creating a cache payload vector from a string.
+BlobDataPtr CreatePayload(const std::string& input) {
+  return new BlobData(input);

[Wez, 2016/04/21 00:55:02]

nit: Use make_scoped_refptr(...) here.

[Kevin M, 2016/04/21 21:24:08]

Used BlobDataPtr instead, so that we're consistent with use of typedefs.

+}
+
+MATCHER_P(BlobDataEqual, expected, "") {
+  return expected->data == arg->data;
+}
+
+class MockBlobReceiverDelegate : public BlobChannelReceiver::Delegate {

[Wez, 2016/04/21 00:55:02]

Is there any value in this being a mock, since there is nothing in it to mock?

[Kevin M, 2016/04/21 21:24:08]

It will eventually have mock methods, once we have add client Get(). But for
now, yeah - it's not a mock, so I'll call it a NullBlobReceiverDelegate.

I use the subclass for promoting OnBlobReceived to public visibility.

+ public:
+  using BlobChannelReceiver::Delegate::OnBlobReceived;
+};
+
+class BlobChannelReceiverTest : public testing::Test {
+ public:
+  BlobChannelReceiverTest()
+      : delegate_(new testing::StrictMock<MockBlobReceiverDelegate>),
+        cache_(new testing::StrictMock<MockBlobCache>),
+        blob_receiver_(new BlobChannelReceiver(base::WrapUnique(cache_),
+                                               base::WrapUnique(delegate_)))
+
+  {}
+
+  ~BlobChannelReceiverTest() override {}
+
+  testing::StrictMock<MockBlobReceiverDelegate>* delegate_;
+  testing::StrictMock<MockBlobCache>* cache_;
+  std::unique_ptr<BlobChannelReceiver> blob_receiver_;
+};
+
+TEST_F(BlobChannelReceiverTest, TestGetFromCache) {

[Wez, 2016/04/21 00:55:02]

Don't include Test in a test's name; we already know it's a test, 'cos the
fixture name ends in Test. ;)  Name it based on what it tests, e.g. this might
be GetKnownBlob.

[Kevin M, 2016/04/21 21:24:08]

Done.

+  auto payload = CreatePayload(kBlobPayload1);
+  EXPECT_CALL(*cache_, Get(kBlobId1)).WillOnce(Return(payload));
+
+  auto result = blob_receiver_->Get(kBlobId1);
+
+  ASSERT_NE(result.get(), nullptr);

[Wez, 2016/04/21 00:55:02]

nit: No need for this, since dereferencing result will crash if it's null.

[Kevin M, 2016/04/21 21:24:08]

Acknowledged.

+  EXPECT_EQ(result->data, payload->data);
+}
+
+TEST_F(BlobChannelReceiverTest, TestGetFromTransport) {

[Wez, 2016/04/21 00:55:02]

This isn't really getting-from-transport, but testing that OnBlobReceived() has
the desired effect on the cache, right?

[Kevin M, 2016/04/21 21:24:08]

Yes, renamed.

+  auto payload = CreatePayload(kBlobPayload1);
+
+  EXPECT_CALL(*cache_, Put(kBlobId1, BlobDataEqual(payload)));
+  delegate_->OnBlobReceived(kBlobId1, payload);
+
+  EXPECT_CALL(*cache_, Get(kBlobId1)).WillOnce(Return(payload));
+  auto result = blob_receiver_->Get(kBlobId1);

[Wez, 2016/04/21 00:55:02]

If you used a real BlobCache instead of a mock then this test would make more
sense; you'd call OnBlobReceived() and verify that Get() returns the correct
result.  You'd also try Get() before OnBlobReceived() to verify that the cache
is clean beforehand.

The preceding test would instead look like:

ASSERT_FALSE(cache.Contains)
cache.Put
EXPECT_EQ(receiver.Get)

[Kevin M, 2016/04/21 21:24:08]

Done.

[Wez, 2016/04/21 22:52:50]

After suggesting this, I realised that an advantage of the mock approach is that
you can verify that Put() gets called exactly-once. 

You might consider mixing mock + in-memory impl so that you have mock-able
methods you can verify get called directly, but which also forward-on to the
"real" underlying impl. On the other hand the test is so simple that your
previous mock-based approach was pretty simple.

Up to you!

+
+  ASSERT_NE(nullptr, result.get());
+  EXPECT_EQ(payload->data, result->data);
+}
+
+TEST_F(BlobChannelReceiverTest, TestGetUnknownBlob) {
+  EXPECT_CALL(*cache_, Get(kBlobId1)).WillOnce(Return(nullptr));
+  auto result = blob_receiver_->Get(kBlobId1);
+
+  ASSERT_EQ(nullptr, result.get());
+}
+
+}  // namespace
+}  // namespace blimp