minor prefactoring split out from later diffs

Summary: Also print the inode path if the assertions in EXPECT_FILE_INODE fail.

Reviewed By: simpkins

Differential Revision: D7035517

fbshipit-source-id: 6c50acb588d1c985c7e7a1586c06f04a657698f9

eden/fs/inodes/FileInode.cpp

@@ -526,9 +526,9 @@ std::tuple<FileInodePtr, FileInode::FileHandlePtr> FileInode::create(
     PathComponentPiece name,
     mode_t mode,
     folly::File&& file,
-    timespec ctime) {
+    timespec timestamp) {
   // The FileInode is in MATERIALIZED_IN_OVERLAY state.
-  auto inode = FileInodePtr::makeNew(ino, parentInode, name, mode, ctime);
+  auto inode = FileInodePtr::makeNew(ino, parentInode, name, mode, timestamp);
 
   auto state = LockedState{inode};
   state.incOpenCount();
@@ -559,9 +559,9 @@ FileInode::FileInode(
     TreeInodePtr parentInode,
     PathComponentPiece name,
     mode_t mode,
-    timespec ctime)
+    timespec timestamp)
     : InodeBase(ino, mode_to_dtype(mode), std::move(parentInode), name),
-      state_(folly::in_place, this, mode, ctime) {}
+      state_(folly::in_place, this, mode, timestamp) {}
 
 folly::Future<Dispatcher::Attr> FileInode::getattr() {
   // Future optimization opportunity: right now, if we have not already

eden/fs/inodes/FileInode.h

@@ -47,7 +47,7 @@ class FileInode : public InodeBase {
       PathComponentPiece name,
       mode_t mode,
       folly::File&& file,
-      timespec ctime);
+      timespec timestamp);
 
   /**
    * If hash is none, this opens the file in the overlay and leaves the inode
@@ -71,7 +71,7 @@ class FileInode : public InodeBase {
       TreeInodePtr parentInode,
       PathComponentPiece name,
       mode_t mode,
-      timespec ctime);
+      timespec timestamp);
 
   folly::Future<Dispatcher::Attr> getattr() override;
 

eden/fs/inodes/TreeInode.h

@@ -280,20 +280,27 @@ class TreeInode : public InodeBase {
     explicit CreateResult(const EdenMount* mount);
   };
 
+  /**
+   * Construct a TreeInode from a source control tree.
+   */
   TreeInode(
       InodeNumber ino,
       TreeInodePtr parent,
       PathComponentPiece name,
       std::shared_ptr<const Tree>&& tree);
 
-  /// Construct an inode that only has backing in the Overlay area
+  /**
+   * Construct an inode that only has backing in the Overlay area.
+   */
   TreeInode(
       InodeNumber ino,
       TreeInodePtr parent,
       PathComponentPiece name,
       Dir&& dir);
 
-  /// Constructors for the root TreeInode
+  /**
+   * Construct the root TreeInode from a source control commit's root.
+   */
   TreeInode(EdenMount* mount, std::shared_ptr<const Tree>&& tree);
   TreeInode(EdenMount* mount, Dir&& tree);
 

eden/fs/inodes/test/CheckoutTest.cpp

@@ -399,21 +399,18 @@ void testModifyFile(
   EXPECT_FILE_INODE(postInode, contents2, perms2);
 }
 
-void testModifyFile(
-    folly::StringPiece path,
-    LoadBehavior loadType,
-    folly::StringPiece contents1,
-    folly::StringPiece contents2) {
-  testModifyFile(path, loadType, contents1, 0644, contents2, 0644);
-}
-
 void runModifyFileTests(folly::StringPiece path) {
   // Modify just the file contents, but not the permissions
   for (auto loadType : kAllLoadTypes) {
     SCOPED_TRACE(folly::to<string>(
         "contents change, path ", path, " load type ", loadType));
     testModifyFile(
-        path, loadType, "contents v1", "updated file contents\nextra stuff\n");
+        path,
+        loadType,
+        "contents v1",
+        0644,
+        "updated file contents\nextra stuff\n",
+        0644);
   }
 
   // Modify just the permissions, but not the contents
@@ -512,7 +509,8 @@ void testModifyConflict(
   postInode.reset();
   testMount.remount();
   postInode = testMount.getFileInode(path);
-  auto ddddInode = testMount.getFileInode("a/b/dddd.c");
+  auto ddddPath = "a/b/dddd.c";
+  auto ddddInode = testMount.getFileInode(ddddPath);
   switch (checkoutMode) {
     case CheckoutMode::FORCE:
       EXPECT_FILE_INODE(postInode, contents2, perms2);

eden/fs/testharness/TestChecks.h

@@ -18,13 +18,15 @@
 /**
  * Check that a FileInode has the expected contents and permissions.
  */
-#define EXPECT_FILE_INODE(fileInode, expectedData, expectedPerms)   \
-  do {                                                              \
-    EXPECT_EQ(                                                      \
-        expectedData,                                               \
-        folly::StringPiece{                                         \
-            (fileInode)->readAll().get(std::chrono::seconds(20))}); \
-    EXPECT_EQ(                                                      \
-        folly::sformat("{:#o}", (expectedPerms)),                   \
-        folly::sformat("{:#o}", (fileInode)->getPermissions()));    \
+#define EXPECT_FILE_INODE(fileInode, expectedData, expectedPerms)  \
+  do {                                                             \
+    EXPECT_EQ(                                                     \
+        expectedData,                                              \
+        folly::StringPiece{                                        \
+            (fileInode)->readAll().get(std::chrono::seconds(20))}) \
+        << " for inode path " << (fileInode)->getLogPath();        \
+    EXPECT_EQ(                                                     \
+        folly::sformat("{:#o}", (expectedPerms)),                  \
+        folly::sformat("{:#o}", (fileInode)->getPermissions()))    \
+        << " for inode path " << (fileInode)->getLogPath();        \
   } while (0)