Clean up ASSERT/EXPECT in file_util_unittest.cc.

base/file_util_unittest.cc

Index: base/file_util_unittest.cc
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index cbe8f26ea0545bdc06dc6c29afc026ae3a74fe5a..910e84c493f3f98c406e5ce83beaf8650cf37bde 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -664,28 +664,27 @@ TEST_F(FileUtilTest, CreateAndReadSymlinks) {
   ASSERT_TRUE(file_util::CreateSymbolicLink(link_to, link_from))
     << "Failed to create file symlink.";
 
-  // If we created the link properly, we should be able to read the
-  // contents through it.
+  // If we created the link properly, we should be able to read the contents
+  // through it.
   std::wstring contents = ReadTextFile(link_from);
-  ASSERT_EQ(contents, bogus_content);
+  EXPECT_EQ(contents, bogus_content);

[Mark Mentovai, 2013/07/02 20:24:53]

ASSERT_EQ and EXPECT_EQ take arguments as “expected, actual”. On _EQ comparison
failure, the message identifies which value was the expected one and which was
the actual one, so it’s nice to get this right. Since |bogus_content| is just a
const, and the “actual” here is what’s returned from ReadTextFile, |contents|, I
think the arguments should be switched.

[gavinp, 2013/07/03 04:39:58]

Done.

 
   FilePath result;
-  ASSERT_TRUE(file_util::ReadSymbolicLink(link_from, &result));
-  ASSERT_EQ(link_to.value(), result.value());
+  EXPECT_TRUE(file_util::ReadSymbolicLink(link_from, &result));
+  EXPECT_EQ(link_to.value(), result.value());

[Mark Mentovai, 2013/07/02 20:24:53]

This uses “result” which won’t be set if ReadSymbolicLink failed, so I think
ASSERT_TRUE is correct on the preceding line.

[gavinp, 2013/07/03 04:39:58]

Done.

 
   // Link to a directory.
   link_from = temp_dir_.path().Append(FPL("from_dir"));
   link_to = temp_dir_.path().Append(FPL("to_dir"));
-  file_util::CreateDirectory(link_to);
-
+  ASSERT_TRUE(file_util::CreateDirectory(link_to));
   ASSERT_TRUE(file_util::CreateSymbolicLink(link_to, link_from))
     << "Failed to create directory symlink.";
 
   // Test failures.
-  ASSERT_FALSE(file_util::CreateSymbolicLink(link_to, link_to));
-  ASSERT_FALSE(file_util::ReadSymbolicLink(link_to, &result));
+  EXPECT_FALSE(file_util::CreateSymbolicLink(link_to, link_to));
+  EXPECT_FALSE(file_util::ReadSymbolicLink(link_to, &result));
   FilePath missing = temp_dir_.path().Append(FPL("missing"));
-  ASSERT_FALSE(file_util::ReadSymbolicLink(missing, &result));
+  EXPECT_FALSE(file_util::ReadSymbolicLink(missing, &result));
 }
 
 // The following test of NormalizeFilePath() require that we create a symlink.
@@ -705,20 +704,19 @@ TEST_F(FileUtilTest, NormalizeFilePathSymlinks) {
     << "Failed to create file symlink.";
 
   // Check that NormalizeFilePath sees the link.
-  ASSERT_TRUE(file_util::NormalizeFilePath(link_from, &normalized_path));
-  ASSERT_TRUE(link_to != link_from);
-  ASSERT_EQ(link_to.BaseName().value(), normalized_path.BaseName().value());
-  ASSERT_EQ(link_to.BaseName().value(), normalized_path.BaseName().value());
+  EXPECT_TRUE(file_util::NormalizeFilePath(link_from, &normalized_path));
+  EXPECT_NE(link_from, link_to);
+  EXPECT_EQ(link_to.BaseName().value(), normalized_path.BaseName().value());

[Mark Mentovai, 2013/07/02 20:24:53]

normalized_path won’t be set unless NormalizeFilePath worked, so that one should
stay an ASSERT.

But I’d call NormalizeFilePath after the EXPECT_NE(link_from, link_to) check,
since the EXPECT_NE check can still run regardless of the status of the
ASSERT_TRUE. And I’d move the declaration of normalized_path down to the line
immediately before where it’s used.

[gavinp, 2013/07/03 04:39:58]

Done and done.

+  EXPECT_EQ(link_to.BaseName().value(), normalized_path.BaseName().value());
 
   // Link to a directory.
   link_from = temp_dir_.path().Append(FPL("from_dir"));
   link_to = temp_dir_.path().Append(FPL("to_dir"));
-  file_util::CreateDirectory(link_to);
-
+  ASSERT_TRUE(file_util::CreateDirectory(link_to));
   ASSERT_TRUE(file_util::CreateSymbolicLink(link_to, link_from))
     << "Failed to create directory symlink.";
 
-  ASSERT_FALSE(file_util::NormalizeFilePath(link_from, &normalized_path))
+  EXPECT_FALSE(file_util::NormalizeFilePath(link_from, &normalized_path))
     << "Links to directories should return false.";
 
   // Test that a loop in the links causes NormalizeFilePath() to return false.
@@ -730,7 +728,7 @@ TEST_F(FileUtilTest, NormalizeFilePathSymlinks) {
     << "Failed to create loop symlink b.";
 
   // Infinite loop!
-  ASSERT_FALSE(file_util::NormalizeFilePath(link_from, &normalized_path));
+  EXPECT_FALSE(file_util::NormalizeFilePath(link_from, &normalized_path));
 }
 #endif  // defined(OS_POSIX)