From 13dc5d1d51ce7a1b0428014192ae926295b8e31e Mon Sep 17 00:00:00 2001 From: Chad Austin Date: Tue, 24 Apr 2018 18:18:37 -0700 Subject: [PATCH] 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 | 8 ++++---- eden/fs/inodes/FileInode.h | 4 ++-- eden/fs/inodes/TreeInode.h | 11 +++++++++-- eden/fs/inodes/test/CheckoutTest.cpp | 18 ++++++++---------- eden/fs/testharness/TestChecks.h | 20 +++++++++++--------- 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/eden/fs/inodes/FileInode.cpp b/eden/fs/inodes/FileInode.cpp index 713a53a9a9..7c4faf9beb 100644 --- a/eden/fs/inodes/FileInode.cpp +++ b/eden/fs/inodes/FileInode.cpp @@ -526,9 +526,9 @@ std::tuple 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 FileInode::getattr() { // Future optimization opportunity: right now, if we have not already diff --git a/eden/fs/inodes/FileInode.h b/eden/fs/inodes/FileInode.h index 0eae38f235..6cba47dfc6 100644 --- a/eden/fs/inodes/FileInode.h +++ b/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 getattr() override; diff --git a/eden/fs/inodes/TreeInode.h b/eden/fs/inodes/TreeInode.h index 22dd9d8078..9107190b8c 100644 --- a/eden/fs/inodes/TreeInode.h +++ b/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&& 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&& tree); TreeInode(EdenMount* mount, Dir&& tree); diff --git a/eden/fs/inodes/test/CheckoutTest.cpp b/eden/fs/inodes/test/CheckoutTest.cpp index 0be102ff72..5a4b9cf071 100644 --- a/eden/fs/inodes/test/CheckoutTest.cpp +++ b/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( "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); diff --git a/eden/fs/testharness/TestChecks.h b/eden/fs/testharness/TestChecks.h index 0cea61fbbb..31c8142a28 100644 --- a/eden/fs/testharness/TestChecks.h +++ b/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)