Linux: Change ShellIntegration code to use GetXDGDirectory().

chrome/browser/shell_integration_linux_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 "chrome/browser/shell_integration_linux.h"
 
 #include <algorithm>
 #include <cstdlib>
 #include <map>
 #include <vector>
 
 #include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_path_override.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 
 namespace shell_integration_linux {
 
 namespace {
 
 // Provides mock environment variables values based on a stored map.
 class MockEnvironment : public base::Environment {
  public:
   MockEnvironment() {}
@@ skipping 52 matching lines @@
 
 }  // namespace
 
 TEST(ShellIntegrationTest, GetDataWriteLocation) {
   base::MessageLoop message_loop;
   content::TestBrowserThread file_thread(BrowserThread::FILE, &message_loop);
 
   // 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) {
   base::MessageLoop message_loop;
   content::TestBrowserThread file_thread(BrowserThread::FILE, &message_loop);
 
   // 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(
         FilePathsToStrings(GetDataSearchLocations(&env)),
         ElementsAre("/user/path",
                     "/system/path/1",
                     "/system/path/2"));
   }
 
   // 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)),
         ElementsAre("/home/user/.local/share",
                     "/system/path/1",
                     "/system/path/2"));
   }
 
   // Test that if neither $XDG_DATA_HOME nor $HOME are specified, it still
   // succeeds.
   {
     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)),
         ElementsAre("/user/path",
                     "/usr/local/share",
                     "/usr/share"));
   }
 }
 
 TEST(ShellIntegrationTest, GetExistingShortcutLocations) {
@@ skipping 129 matching lines @@
         GetExistingShortcutContents(&env, kTemplateFilepath, &contents));
     EXPECT_EQ(kTestData1, contents);
   }
 
   // Test that it falls back to $HOME/.local/share/applications.
   {
     base::ScopedTempDir temp_dir;
     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(
         temp_dir.path().Append(".local/share/applications")
             .Append(kTemplateFilename),
         kTestData1));
     std::string contents;
     ASSERT_TRUE(
         GetExistingShortcutContents(&env, kTemplateFilepath, &contents));
     EXPECT_EQ(kTestData1, contents);
@@ skipping 304 matching lines @@
 
   for (size_t i = 0; i < arraysize(test_cases); i++) {
     SCOPED_TRACE(i);
     EXPECT_EQ(test_cases[i].expected_output,
               GetDirectoryFileContents(base::ASCIIToUTF16(test_cases[i].title),
                                        test_cases[i].icon_name));
   }
 }
 
 }  // namespace shell_integration_linux