Resubmit of 149313003: Significantly cleans up the ImageWriter Operation class and subclasses.

chrome/browser/extensions/api/image_writer_private/operation_unittest.cc

 // Copyright 2013 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 "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "chrome/browser/extensions/api/image_writer_private/error_messages.h"
 #include "chrome/browser/extensions/api/image_writer_private/operation.h"
@@ skipping 10 matching lines @@
 namespace image_writer {
 
 namespace {
 
 using testing::_;
 using testing::AnyNumber;
 using testing::AtLeast;
 using testing::Gt;
 using testing::Lt;
 
-// This class gives us access to the protected methods of Operation so that we
-// can call them directly.  It also allows us to selectively disable some
-// phases.
+// This class gives us a generic Operation with the ability to set or inspect
+// the current path to the image file.
 class OperationForTest : public Operation {
  public:
-  OperationForTest(base::WeakPtr<OperationManager> manager,
+  OperationForTest(base::WeakPtr<OperationManager> manager_,
                    const ExtensionId& extension_id,
-                   const std::string& storage_unit_id)
-      : Operation(manager, extension_id, storage_unit_id) {}
+                   const std::string& device_path)
+      : Operation(manager_, extension_id, device_path) {}
 
-  virtual void Start() OVERRIDE {
+  virtual void StartImpl() OVERRIDE {}
+
+  // Expose internal stages for testing.
+  void Unzip(const base::Closure& continuation) {
+    Operation::Unzip(continuation);
   }
 
-  void UnzipStart(scoped_ptr<base::FilePath> zip_file) {
-    Operation::UnzipStart(zip_file.Pass());
+  void Write(const base::Closure& continuation) {
+    Operation::Write(continuation);
   }
 
-  void WriteStart() {
-    Operation::WriteStart();
+  void VerifyWrite(const base::Closure& continuation) {
+    Operation::VerifyWrite(continuation);
   }
 
-  void VerifyWriteStart() {
-    Operation::VerifyWriteStart();
+  // Helpers to set-up state for intermediate stages.
+  void SetImagePath(const base::FilePath image_path) {
+    image_path_ = image_path;
   }
 
-  void Finish() {
-    Operation::Finish();
-  }
+  base::FilePath GetImagePath() { return image_path_; }
+
  private:
   virtual ~OperationForTest() {};
 };
 
 class ImageWriterOperationTest : public ImageWriterUnitTestBase {
  protected:
   virtual void SetUp() OVERRIDE {
     ImageWriterUnitTestBase::SetUp();
 
     // Create the zip file.
-    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(),
-                                               &image_file_));
-    ASSERT_TRUE(base::CreateTemporaryFile(&zip_file_));
+    base::FilePath image_dir = temp_dir_.path().AppendASCII("zip");
+    ASSERT_TRUE(base::CreateDirectory(image_dir));
+    ASSERT_TRUE(base::CreateTemporaryFileInDir(image_dir, &image_path_));
 
-    scoped_ptr<char[]> buffer(new char[kTestFileSize]);
-    memset(buffer.get(), kImagePattern, kTestFileSize);
-    file_util::WriteFile(image_file_, buffer.get(), kTestFileSize);
+    FillFile(image_path_, kImagePattern, kTestFileSize);
 
-    zip::Zip(temp_dir_.path(), zip_file_, true);
+    zip_file_ = temp_dir_.path().AppendASCII("test_image.zip");
+    ASSERT_TRUE(zip::Zip(image_dir, zip_file_, true));
   }
 
   virtual void TearDown() OVERRIDE {
     ImageWriterUnitTestBase::TearDown();
   }
 
-  base::ScopedTempDir temp_dir_;
-  base::FilePath image_file_;
+  base::FilePath image_path_;
   base::FilePath zip_file_;
+
+  MockOperationManager manager_;
 };
 
 } // namespace
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
-// Tests a successful unzip.
-TEST_F(ImageWriterOperationTest, Unzip) {
-  MockOperationManager manager;
-
+TEST_F(ImageWriterOperationTest, UnzipNonZipFile) {
   scoped_refptr<OperationForTest> operation(
-      new OperationForTest(manager.AsWeakPtr(),
+      new OperationForTest(manager_.AsWeakPtr(),
                            kDummyExtensionId,
                            test_device_path_.AsUTF8Unsafe()));
 
-  scoped_ptr<base::FilePath> zip_file(new base::FilePath(zip_file_));
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId, _, _)).Times(0);
 
-  // At least one progress report > 0% and < 100%.
-  EXPECT_CALL(manager, OnProgress(kDummyExtensionId,
-                                  image_writer_api::STAGE_UNZIP,
-                                  Lt(100))).Times(AtLeast(1));
-  // At least one progress report at 100%.
-  EXPECT_CALL(manager, OnProgress(kDummyExtensionId,
-                                  image_writer_api::STAGE_UNZIP,
-                                  100)).Times(AtLeast(1));
-  // At least one progress report at 0%.
-  EXPECT_CALL(manager, OnProgress(kDummyExtensionId,
-                                  image_writer_api::STAGE_UNZIP,
-                                  0)).Times(AtLeast(1));
-  // Any number of additional progress calls in later stages.
-  EXPECT_CALL(manager, OnProgress(kDummyExtensionId,
-                                  Gt(image_writer_api::STAGE_UNZIP),
-                                  _)).Times(AnyNumber());
-  // One completion call.
-  EXPECT_CALL(manager, OnComplete(kDummyExtensionId)).Times(1);
-  // No errors
-  EXPECT_CALL(manager, OnError(_, _, _, _)).Times(0);
+  operation->SetImagePath(test_image_path_);
 
-  content::BrowserThread::PostTask(content::BrowserThread::FILE,
-                                   FROM_HERE,
-                                   base::Bind(&OperationForTest::UnzipStart,
-                                              operation,
-                                              base::Passed(&zip_file)));
+  operation->Start();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(
+          &OperationForTest::Unzip, operation, base::Bind(&base::DoNothing)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(ImageWriterOperationTest, UnzipZipFile) {
+  scoped_refptr<OperationForTest> operation(
+      new OperationForTest(manager_.AsWeakPtr(),
+                           kDummyExtensionId,
+                           test_device_path_.AsUTF8Unsafe()));
+
+  EXPECT_CALL(manager_, OnError(kDummyExtensionId, _, _, _)).Times(0);
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_UNZIP, _))
+      .Times(AtLeast(1));
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_UNZIP, 0))
+      .Times(AtLeast(1));
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_UNZIP, 100))
+      .Times(AtLeast(1));
+
+  operation->SetImagePath(zip_file_);
+
+  operation->Start();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(
+          &OperationForTest::Unzip, operation, base::Bind(&base::DoNothing)));
 
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(base::ContentsEqual(image_file_, test_device_path_));
+  EXPECT_TRUE(base::ContentsEqual(image_path_, operation->GetImagePath()));
+}
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+// Chrome OS tests don't actually write to the disk because that's handled by

[tbarzic, 2014/02/18 22:13:14]

nit: update the comment

[Drew Haven, 2014/02/19 00:49:19]

I just moved this comment to a compiler condition around the writing check
itself.  ChromeOS code still works fine for the rest of the test.

[tbarzic, 2014/02/19 01:20:16]

yeah, we only have linux and cros implementation... do'h

+// the DBUS process.
+TEST_F(ImageWriterOperationTest, WriteImageToDevice) {
+
+  scoped_refptr<OperationForTest> operation(
+      new OperationForTest(manager_.AsWeakPtr(),
+                           kDummyExtensionId,
+                           test_device_path_.AsUTF8Unsafe()));
+
+  EXPECT_CALL(manager_, OnError(kDummyExtensionId, _, _, _)).Times(0);
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_WRITE, _))
+      .Times(AtLeast(1));
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_WRITE, 0))
+      .Times(AtLeast(1));
+  EXPECT_CALL(manager_,
+              OnProgress(kDummyExtensionId, image_writer_api::STAGE_WRITE, 100))
+      .Times(AtLeast(1));
+
+  operation->SetImagePath(test_image_path_);
+
+  operation->Start();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(
+          &OperationForTest::Write, operation, base::Bind(&base::DoNothing)));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(base::ContentsEqual(test_image_path_, test_device_path_));
 }
 #endif
 
-TEST_F(ImageWriterOperationTest, Creation) {
-  MockOperationManager manager;
-  scoped_refptr<Operation> op(
-      new OperationForTest(manager.AsWeakPtr(),
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+// Chrome OS doesn't support verification in the ImageBurner, so these two tests
+// are skipped.
+
+TEST_F(ImageWriterOperationTest, VerifyFileSuccess) {
+  scoped_refptr<OperationForTest> operation(
+      new OperationForTest(manager_.AsWeakPtr(),
                            kDummyExtensionId,
                            test_device_path_.AsUTF8Unsafe()));
 
+  EXPECT_CALL(manager_, OnError(kDummyExtensionId, _, _, _)).Times(0);
+  EXPECT_CALL(
+      manager_,
+      OnProgress(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, _))
+      .Times(AtLeast(1));
+  EXPECT_CALL(
+      manager_,
+      OnProgress(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, 0))
+      .Times(AtLeast(1));
+  EXPECT_CALL(
+      manager_,
+      OnProgress(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, 100))
+      .Times(AtLeast(1));
+
+  FillFile(test_device_path_, kImagePattern, kTestFileSize);
+  operation->SetImagePath(test_image_path_);
+
+  operation->Start();
+  content::BrowserThread::PostTask(content::BrowserThread::FILE,
+                                   FROM_HERE,
+                                   base::Bind(&OperationForTest::VerifyWrite,
+                                              operation,
+                                              base::Bind(&base::DoNothing)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(ImageWriterOperationTest, VerifyFileFailure) {
+  scoped_refptr<OperationForTest> operation(
+      new OperationForTest(manager_.AsWeakPtr(),
+                           kDummyExtensionId,
+                           test_device_path_.AsUTF8Unsafe()));
+
+  EXPECT_CALL(
+      manager_,
+      OnError(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, _, _))
+      .Times(1);
+  EXPECT_CALL(
+      manager_,
+      OnProgress(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, _))
+      .Times(AnyNumber());
+  EXPECT_CALL(
+      manager_,
+      OnProgress(kDummyExtensionId, image_writer_api::STAGE_VERIFYWRITE, 100))
+      .Times(0);
+
+  FillFile(test_device_path_, kDevicePattern, kTestFileSize);
+  operation->SetImagePath(test_image_path_);
+
+  operation->Start();
+  content::BrowserThread::PostTask(content::BrowserThread::FILE,
+                                   FROM_HERE,
+                                   base::Bind(&OperationForTest::VerifyWrite,
+                                              operation,
+                                              base::Bind(&base::DoNothing)));
+
+  base::RunLoop().RunUntilIdle();
+}
+#endif
+
+// Tests that on creation the operation has the expected state.
+TEST_F(ImageWriterOperationTest, Creation) {
+  scoped_refptr<Operation> op(
+      new OperationForTest(manager_.AsWeakPtr(),
+                           kDummyExtensionId,
+                           test_device_path_.AsUTF8Unsafe()));
+
   EXPECT_EQ(0, op->GetProgress());
   EXPECT_EQ(image_writer_api::STAGE_UNKNOWN, op->GetStage());
 }
 
 }  // namespace image_writer
 }  // namespace extensions