Prettify output from chrome.fileSystem.getDisplayPath for POSIX profile directories

chrome/browser/extensions/api/file_system/file_system_apitest.cc

 // 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 "base/file_util.h"
 #include "base/path_service.h"
 #include "chrome/browser/extensions/api/file_system/file_system_api.h"
 #include "chrome/browser/extensions/platform_app_browsertest_util.h"
 
 using extensions::FileSystemChooseFileFunction;
@@ skipping 30 matching lines @@
 };
 
 IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiGetDisplayPath) {
   FilePath test_file = test_root_folder_.AppendASCII("gold.txt");
   FileSystemChooseFileFunction::SkipPickerAndAlwaysSelectPathForTest(
       &test_file);
   ASSERT_TRUE(RunPlatformAppTest("api_test/file_system/get_display_path"))
       << message_;
 }
 
+IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiGetDisplayPathPrettify) {
 #if defined(OS_WIN)
-IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiGetDisplayPathPrettify) {
-  ASSERT_TRUE(PathService::OverrideAndCreateIfNeeded(base::DIR_PROFILE,
+  int override = base::DIR_PROFILE;
+#elif defined(OS_POSIX)
+  int override = base::DIR_HOME;
+#endif

[benwells, 2012/07/10 23:57:49]

Again, I don't like relying on OS_POSIX == !OS_WIN

[thorogood, 2012/07/11 00:30:24]

I've guarded this test by a define again.

+  ASSERT_TRUE(PathService::OverrideAndCreateIfNeeded(override,
       test_root_folder_, false));
 
   FilePath test_file = test_root_folder_.AppendASCII("gold.txt");
   FileSystemChooseFileFunction::SkipPickerAndAlwaysSelectPathForTest(
       &test_file);
   ASSERT_TRUE(RunPlatformAppTest(
       "api_test/file_system/get_display_path_prettify")) << message_;

[Mihai Parparita -not on Chrome, 2012/07/11 00:21:10]

Sorry for not catching this in the other review, but for tests like this one
where you're just checking the result of an ExtensionFunction invocation, it's
less overhead* to do it directly in C++.  There's a bunch of helpers in the
extension_function_test_utils namespace, see
http://code.google.com/p/chromium/source/search?q=%3A%3ARunFunction+file%3Atest
for sample uses.

* both in that the tests will run faster, and in that you don't have to create a
new app/extension manifest, background page, etc. for the test.

[thorogood, 2012/07/11 04:07:45]

Thanks for the insight! I've had a play with it, but I'm not sure if this is the
right approach here. The getDisplayPath function only makes sense on an
initialized FileEntry, which we only get from calling another API function which
builds it and registers an isolated filesystem, so I feel like it's still neater
to test via a manifest.

Some of the other tests here that don't depend on other API calls for their
results might be better candidates.

[Mihai Parparita -not on Chrome, 2012/07/11 05:32:28]

Fair enough.

 }
-#endif
 
 IN_PROC_BROWSER_TEST_F(FileSystemApiTest, FileSystemApiOpenExistingFileTest) {
   FilePath test_file = TempFilePath("open_existing.txt", true);
   ASSERT_FALSE(test_file.empty());
   FileSystemChooseFileFunction::SkipPickerAndAlwaysSelectPathForTest(
       &test_file);
   ASSERT_TRUE(RunPlatformAppTest("api_test/file_system/open_existing"))
       << message_;
 }
 
@@ skipping 108 matching lines @@
 
 IN_PROC_BROWSER_TEST_F(FileSystemApiTest,
     FileSystemApiGetWritableWithWriteTest) {
   FilePath test_file = TempFilePath("writable.txt", true);
   ASSERT_FALSE(test_file.empty());
   FileSystemChooseFileFunction::SkipPickerAndAlwaysSelectPathForTest(
       &test_file);
   ASSERT_TRUE(RunPlatformAppTest(
       "api_test/file_system/get_writable_file_entry_with_write")) << message_;
 }