Linux: Change ShellIntegration code to use GetXDGDirectory().

chrome/browser/shell_integration_linux_unittest.cc

Index: chrome/browser/shell_integration_linux_unittest.cc
diff --git a/chrome/browser/shell_integration_linux_unittest.cc b/chrome/browser/shell_integration_linux_unittest.cc
index 05438f06157d7197f06bc8947cf492c07d543e7f..60fd082e6bb4024235e6554ebee82c5b99023a34 100644
--- a/chrome/browser/shell_integration_linux_unittest.cc
+++ b/chrome/browser/shell_integration_linux_unittest.cc
@@ -27,6 +27,7 @@
 #include "url/gurl.h"
 
 using content::BrowserThread;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 
 namespace shell_integration_linux {
@@ -99,28 +100,25 @@ TEST(ShellIntegrationTest, GetDataWriteLocation) {
   // Test that it returns $XDG_DATA_HOME.
   {
     MockEnvironment env;
-    env.Set("HOME", "/home/user");
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           base::FilePath("/home/user"),
+                                           true /* absolute? */,
+                                           false /* create? */);
     env.Set("XDG_DATA_HOME", "/user/path");
-    base::FilePath path;
-    ASSERT_TRUE(GetDataWriteLocation(&env, &path));
+    base::FilePath path = GetDataWriteLocation(&env);
     EXPECT_EQ("/user/path", path.value());
   }
 
   // Test that $XDG_DATA_HOME falls back to $HOME/.local/share.
   {
     MockEnvironment env;
-    env.Set("HOME", "/home/user");
-    base::FilePath path;
-    ASSERT_TRUE(GetDataWriteLocation(&env, &path));
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           base::FilePath("/home/user"),
+                                           true /* absolute? */,
+                                           false /* create? */);
+    base::FilePath path = GetDataWriteLocation(&env);
     EXPECT_EQ("/home/user/.local/share", path.value());
   }
-
-  // Test that if neither $XDG_DATA_HOME nor $HOME are specified, it fails.
-  {
-    MockEnvironment env;
-    base::FilePath path;
-    ASSERT_FALSE(GetDataWriteLocation(&env, &path));
-  }
 }
 
 TEST(ShellIntegrationTest, GetDataSearchLocations) {
@@ -130,7 +128,10 @@ TEST(ShellIntegrationTest, GetDataSearchLocations) {
   // Test that it returns $XDG_DATA_HOME + $XDG_DATA_DIRS.
   {
     MockEnvironment env;
-    env.Set("HOME", "/home/user");
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           base::FilePath("/home/user"),
+                                           true /* absolute? */,
+                                           false /* create? */);
     env.Set("XDG_DATA_HOME", "/user/path");
     env.Set("XDG_DATA_DIRS", "/system/path/1:/system/path/2");
     EXPECT_THAT(
@@ -143,7 +144,10 @@ TEST(ShellIntegrationTest, GetDataSearchLocations) {
   // Test that $XDG_DATA_HOME falls back to $HOME/.local/share.
   {
     MockEnvironment env;
-    env.Set("HOME", "/home/user");
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           base::FilePath("/home/user"),
+                                           true /* absolute? */,
+                                           false /* create? */);
     env.Set("XDG_DATA_DIRS", "/system/path/1:/system/path/2");
     EXPECT_THAT(
         FilePathsToStrings(GetDataSearchLocations(&env)),
@@ -157,16 +161,20 @@ TEST(ShellIntegrationTest, GetDataSearchLocations) {
   {
     MockEnvironment env;
     env.Set("XDG_DATA_DIRS", "/system/path/1:/system/path/2");
-    EXPECT_THAT(
-        FilePathsToStrings(GetDataSearchLocations(&env)),
-        ElementsAre("/system/path/1",
-                    "/system/path/2"));
+    std::vector<std::string> results =
+        FilePathsToStrings(GetDataSearchLocations(&env));
+    ASSERT_EQ(3U, results.size());

[Matt Giuca, 2015/01/18 23:52:10]

I'm not sure why you changed this from a nice ElementsAre to a more awkward
length-and-individual-elements test. This new test is also not testing the
order. Is it because the output changed so now the first element of the result
is ""?

That's kind of unfortunate (part of the point of this test is to check that if
there is no $HOME, that GetDataSearchLocations just ignores it). But as it is
unlikely that there will be no $HOME, I think it's OK to change the behaviour.

But I do think the test should properly assert the full output of
GetDataSearchLocations. So:

EXPECT_THAT(
    ...,
    ElementsAre("",
                "/system/path/1",
                "/system/path/2"));

is my preference.

+    EXPECT_THAT(results, Contains("/system/path/1"));
+    EXPECT_THAT(results, Contains("/system/path/2"));
   }
 
   // Test that $XDG_DATA_DIRS falls back to the two default paths.
   {
     MockEnvironment env;
-    env.Set("HOME", "/home/user");
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           base::FilePath("/home/user"),
+                                           true /* absolute? */,
+                                           false /* create? */);
     env.Set("XDG_DATA_HOME", "/user/path");
     EXPECT_THAT(
         FilePathsToStrings(GetDataSearchLocations(&env)),
@@ -316,7 +324,10 @@ TEST(ShellIntegrationTest, GetExistingShortcutContents) {
     ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
     MockEnvironment env;
-    env.Set("HOME", temp_dir.path().value());
+    base::ScopedPathOverride home_override(base::DIR_HOME,
+                                           temp_dir.path(),
+                                           true /* absolute? */,
+                                           false /* create? */);
     ASSERT_TRUE(base::CreateDirectory(
         temp_dir.path().Append(".local/share/applications")));
     ASSERT_TRUE(WriteString(