Create VisualElementsManifest.xml from template -- install VisualElements in the version directory

chrome/installer/setup/install_unittest.cc

Index: chrome/installer/setup/install_unittest.cc
diff --git a/chrome/installer/setup/install_unittest.cc b/chrome/installer/setup/install_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..44c0c7b066622fa777736fd3ed3a5026919fd766
--- /dev/null
+++ b/chrome/installer/setup/install_unittest.cc
@@ -0,0 +1,105 @@
+// Copyright (c) 2012 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 <string>
+
+#include "base/file_util.h"
+#include "base/platform_file.h"
+#include "base/scoped_temp_dir.h"
+#include "base/version.h"
+#include "chrome/installer/setup/install.h"
+#include "chrome/installer/setup/setup_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class InstallTest : public testing::Test {
+ protected:
+  virtual void SetUp() {

[grt (UTC plus 2), 2012/05/04 13:19:14]

SetUp() OVERRIDE {

[gab, 2012/05/04 17:26:35]

Interesting, I copied the format from setup_util_unittest.cc... guess it's wrong
:)!

+    // Create a temp directory for testing.
+    ASSERT_TRUE(test_dir_.CreateUniqueTempDir());
+  }
+
+  virtual void TearDown() {

[grt (UTC plus 2), 2012/05/04 13:19:14]

TearDown() OVERRIDE {

[gab, 2012/05/04 17:26:35]

Done.

+    // Clean up test directory manually so we can fail if it leaks.
+    ASSERT_TRUE(test_dir_.Delete());
+  }
+
+  // Initializes InstallTest variables for tests testing
+  // installer::CreateVisualElementsManifest
+  void InitVisualElementsTest(bool create_elements_folder) {

[grt (UTC plus 2), 2012/05/04 13:19:14]

move everything in this function through line 43 into SetUp.

[gab, 2012/05/04 17:26:35]

Done.

+    work_dir_ = test_dir_.path();
+    ASSERT_TRUE(file_util::PathExists(work_dir_));

[grt (UTC plus 2), 2012/05/04 13:19:14]

there's no need to check this since the assert on line 21 assures that this will
never be false.

[gab, 2012/05/04 17:26:35]

Done.

+
+    version_ = Version("0.0.0.0");
+
+    version_dir_ = work_dir_.AppendASCII(version_.GetString());
+    ASSERT_FALSE(file_util::PathExists(version_dir_));

[grt (UTC plus 2), 2012/05/04 13:19:14]

there's no need to check for this since test_dir_ is always created from
scratch.

[gab, 2012/05/04 17:26:35]

Done.

+    EXPECT_TRUE(file_util::CreateDirectory(version_dir_));
+    ASSERT_TRUE(file_util::PathExists(version_dir_));

[grt (UTC plus 2), 2012/05/04 13:19:14]

there's no need to check for this since we assume CreateDirectory has been
tested elsewhere.  if a lack of a version_dir_ is enough to make the test fail
outright, change the EXPECT on line 39 to an ASSERT.

[gab, 2012/05/04 17:26:35]

Done.

+
+    elements_dir_ = version_dir_.Append(installer::kVisualElements);
+    ASSERT_FALSE(file_util::PathExists(elements_dir_));

[grt (UTC plus 2), 2012/05/04 13:19:14]

remove this, too.

[gab, 2012/05/04 17:26:35]

Done.

+    if (create_elements_folder) {

[grt (UTC plus 2), 2012/05/04 13:19:14]

move directory creation into VisualElementsManifestCreated

[gab, 2012/05/04 17:26:35]

Done.

+      EXPECT_TRUE(file_util::CreateDirectory(elements_dir_));
+      ASSERT_TRUE(file_util::PathExists(elements_dir_));
+    }
+  }
+
+  // The path to |test_dir_|.
+  FilePath work_dir_;

[grt (UTC plus 2), 2012/05/04 13:19:14]

remove this and use test_dir_.path() where needed

[gab, 2012/05/04 17:26:35]

Done.

+
+  // A dummy version number used to create the version directory.
+  Version version_;
+
+  // The path to |work_dir_|\|version_|.
+  FilePath version_dir_;
+
+  // The path to |version_dir_|\VisualElements.
+  FilePath elements_dir_;
+
+ private:

[grt (UTC plus 2), 2012/05/04 13:19:14]

i was motivated by a reviewer long ago to make everything in tests protected. 
maybe some other TEST_F will be added that needs access to test_dir_.

[gab, 2012/05/04 17:26:35]

Done.

+  // The temporary directory used to contain the test operations.
+  ScopedTempDir test_dir_;
+};
+
+}  // namespace
+
+// Test that VisualElementsManifest.xml is not created when VisualElements are
+// not present.
+TEST_F(InstallTest, VisualElementsManifestNotCreated) {
+  InitVisualElementsTest(false);
+  EXPECT_TRUE(installer::CreateVisualElementsManifest(work_dir_, version_));
+  EXPECT_FALSE(file_util::PathExists(
+      work_dir_.Append(installer::kVisualElementsManifest)));
+}
+
+// Test that VisualElementsManifest.xml is created with the correct content when
+// VisualElements are present.
+TEST_F(InstallTest, VisualElementsManifestCreated) {
+  InitVisualElementsTest(true);
+  EXPECT_TRUE(installer::CreateVisualElementsManifest(work_dir_, version_));
+  const FilePath manifest_path(

[grt (UTC plus 2), 2012/05/04 13:19:14]

since this path is used by both tests, i think it makes sense to make a
"manifest_path_" member and initialize it in SetUp

[gab, 2012/05/04 17:26:35]

Done.

+      work_dir_.Append(installer::kVisualElementsManifest));
+  EXPECT_TRUE(file_util::PathExists(manifest_path));
+
+  std::string read_manifest;
+  file_util::ReadFileToString(manifest_path, &read_manifest);

[grt (UTC plus 2), 2012/05/04 13:19:14]

ASSERT_TRUE?

[gab, 2012/05/04 17:26:35]

Done.

+
+  static const char kExpectedManifest[] =
+    "<Application>\r\n"
+    "  <VisualElements\r\n"
+    "      DisplayName='Google Chrome'\r\n"
+    "      Logo='0.0.0.0\\VisualElements\\Logo.png'\r\n"
+    "      SmallLogo='0.0.0.0\\VisualElements\\SmallLogo.png'\r\n"
+    "      ForegroundText='light'\r\n"
+    "      BackgroundColor='white'>\r\n"
+    "    <DefaultTile ShowName='allLogos'/>\r\n"
+    "    <SplashScreen Image='0.0.0.0\\VisualElements\\splash-620x300.png'/>"

[grt (UTC plus 2), 2012/05/04 13:19:14]

does this wrap more nicely if you break it just after SplashScreen?

[gab, 2012/05/04 17:26:35]

Ya I guess, wasn't sure where to break it...

+        "\r\n"
+    "  </VisualElements>\r\n"
+    "</Application>";
+
+  EXPECT_STREQ(kExpectedManifest, read_manifest.c_str());
+}