mac: Zip packages when they are selected by the file opener.

chrome/browser/file_select_helper_unittest.cc

Index: chrome/browser/file_select_helper_unittest.cc
diff --git a/chrome/browser/file_select_helper_unittest.cc b/chrome/browser/file_select_helper_unittest.cc
index 5eb0436957f75448ef797d6bd58d9ea11dfbcdee..29ad6c740f1f5beed4ebc517401ed2b61ad52294 100644
--- a/chrome/browser/file_select_helper_unittest.cc
+++ b/chrome/browser/file_select_helper_unittest.cc
@@ -2,10 +2,35 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/path_service.h"
+#include "base/process/launch.h"
 #include "chrome/browser/file_select_helper.h"
+#include "chrome/common/chrome_paths.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(FileSelectHelperTest, IsAcceptTypeValid) {
+class FileSelectHelperTest : public testing::Test {
+ public:
+  FileSelectHelperTest() {}
+
+ protected:
+  virtual void SetUp() OVERRIDE {

[Lei Zhang, 2014/10/09 18:41:55]

nit: we're switching to "override"

[erikchen, 2014/10/09 23:33:36]

Done.

+    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_));
+    data_dir_ = data_dir_.AppendASCII("file_select_helper");
+    ASSERT_TRUE(base::PathExists(data_dir_));

[Lei Zhang, 2014/10/09 18:41:55]

Who creates this?

[erikchen, 2014/10/09 23:33:36]

it's already there. it's a test directory that got added in this CL.

[Lei Zhang, 2014/10/10 01:32:28]

Ah, that's what I get for ignoring the rest of the CL.

+  }
+
+  // The path to input data used in tests.
+  base::FilePath data_dir_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FileSelectHelperTest);
+};
+
+TEST_F(FileSelectHelperTest, IsAcceptTypeValid) {
   EXPECT_TRUE(FileSelectHelper::IsAcceptTypeValid("a/b"));
   EXPECT_TRUE(FileSelectHelper::IsAcceptTypeValid("abc/def"));
   EXPECT_TRUE(FileSelectHelper::IsAcceptTypeValid("abc/*"));
@@ -17,3 +42,39 @@ TEST(FileSelectHelperTest, IsAcceptTypeValid) {
   EXPECT_FALSE(FileSelectHelper::IsAcceptTypeValid("ABC/*"));
   EXPECT_FALSE(FileSelectHelper::IsAcceptTypeValid("abc/def "));
 }
+
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+TEST_F(FileSelectHelperTest, ZipPackage) {

[erikchen, 2014/10/08 20:14:44]

I changed the unit test to zip and unzip the package, and then check that
several specific files did not change. This means that the test is not longer
dependent on the specific zlib compression implementation.

+  // Zip the package.
+  const char* app_name = "CalculatorFake.app";

[Lei Zhang, 2014/10/09 18:41:55]

const char app_name[], otherwise you can potentially change the pointer.

[erikchen, 2014/10/09 23:33:36]

Done.

+  base::FilePath src = data_dir_.Append(app_name);
+  base::FilePath dest = FileSelectHelper::ZipPackage(src);
+  EXPECT_FALSE(dest.empty());

[Lei Zhang, 2014/10/09 18:41:55]

Assert here, otherwise you'd pass an empty path to unzip.

[erikchen, 2014/10/09 23:33:36]

Done. Asserted the line after this as well, since the rest of this test doesn't
make sense of there's no directory at "dest".

+  EXPECT_TRUE(base::PathExists(dest));
+
+  base::ScopedTempDir temp_dir;
+  EXPECT_TRUE(temp_dir.CreateUniqueTempDir());

[Lei Zhang, 2014/10/09 18:41:55]

You should assert true. Otherwise what happens if you pass an empty path to
unzip below?

[erikchen, 2014/10/09 23:33:36]

Done.

+
+  // Unzip the package into a temporary directory.
+  CommandLine cl(base::FilePath("/usr/bin/unzip"));
+  cl.AppendArg(dest.value().c_str());
+  cl.AppendArg("-d");
+  cl.AppendArg(temp_dir.path().value().c_str());
+  std::string output;
+  EXPECT_TRUE(base::GetAppOutput(cl, &output));
+
+  // Verify that several key files haven't changed.
+  const char* files_to_verify[] = {"Contents/Info.plist",
+                                   "Contents/MacOS/Calculator",
+                                   "Contents/_CodeSignature/CodeResources"};
+  unsigned long file_count =

[Lei Zhang, 2014/10/09 18:41:55]

use arraysize() ?

[erikchen, 2014/10/09 23:33:36]

Done.

+      sizeof(files_to_verify) / sizeof(files_to_verify[0]);
+  for (unsigned long i = 0; i < file_count; i++) {

[Lei Zhang, 2014/10/09 18:41:54]

and size_t ?

[erikchen, 2014/10/09 23:33:36]

Done.

+    const char* relative_path = files_to_verify[i];
+    base::FilePath orig_file = src.Append(relative_path);
+    base::FilePath final_file =
+        temp_dir.path().Append(app_name).Append(relative_path);
+    EXPECT_TRUE(base::ContentsEqual(orig_file, final_file));
+  }
+}
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)