Skip to content

Commit

Permalink
Update reftests with small refactor and renames for clarity
Browse files Browse the repository at this point in the history 
Signed-off-by: Daniel Carl Jones <[email protected]>
  • Loading branch information
@dannycjones
dannycjones committed Jan 13, 2025
1 parent ab77aaa commit 7a9bea3
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 33 deletions.
54 changes: 34 additions & 20 deletions mountpoint-s3/tests/reftests/harness.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,26 +253,40 @@ impl Harness {
/// Compared to [compare_contents], this avoids doing a `readdir` before `lookup`, and so tests
/// a different path through the inode code.
pub async fn compare_single_path(&self, idx: usize) {
let inodes = self.reference.list_recursive();
if inodes.is_empty() {
let ref_inodes = self.reference.list_recursive();
if ref_inodes.is_empty() {
return;
}
let (path, node) = &inodes[idx % inodes.len()];
let (path_components, node) = &ref_inodes[idx % ref_inodes.len()];

let mut parent = FUSE_ROOT_INODE;
let mut seen_inos = HashSet::from([FUSE_ROOT_INODE]);
for name in path.iter().take(path.len().saturating_sub(1)) {
let lookup = self.fs.lookup(parent, name.as_ref()).await.unwrap();

let (leaf, ancestors) = path_components
.split_last()
.expect("there should always be at least one path component");
for name in ancestors {
let lookup = self
.fs
.lookup(parent, name.as_ref())
.await
.expect("ancestor must exist in MP fs");
assert_eq!(lookup.attr.kind, FileType::Directory);
let ino = lookup.attr.ino;
assert!(
seen_inos.insert(lookup.attr.ino),
"should not have seen ancestor before"
seen_inos.insert(ino),
"should not have seen ancestor ino {ino} in MP fs before"
);
parent = lookup.attr.ino;
parent = ino;
}

let lookup = self.fs.lookup(parent, path.last().unwrap().as_ref()).await.unwrap();
assert!(seen_inos.insert(lookup.attr.ino), "should not have seen inode before");
let lookup = self
.fs
.lookup(parent, leaf.as_ref())
.await
.expect("directory entry at path should exist in MP fs");
let ino = lookup.attr.ino;
assert!(seen_inos.insert(ino), "should not have seen ino {ino} before");
match node {
Node::Directory { .. } => {
assert_eq!(lookup.attr.kind, FileType::Directory);
Expand Down Expand Up @@ -605,8 +619,8 @@ impl Harness {
) -> BoxFuture<'a, ()> {
async move {
let dir_handle = self.fs.opendir(fs_dir, 0).await.unwrap().fh;
let children = ref_dir.children();
let mut keys = children.keys().cloned().collect::<HashSet<_>>();
let ref_children = ref_dir.children();
let mut unvisited_ref_children = ref_children.keys().cloned().collect::<HashSet<_>>();

let mut reply = DirectoryReply::new(self.readdir_limit);
self.fs.readdir(fs_dir, dir_handle, 0, &mut reply).await.unwrap();
Expand Down Expand Up @@ -648,7 +662,7 @@ impl Harness {
let lkup = self.fs.lookup(fs_dir, &reply.name).await.unwrap();
let attr = lkup.attr;

match children.get(name) {
match ref_children.get(name) {
Some(node) => {
let ref_kind = node.node_type().into();
assert_eq!(
Expand All @@ -672,7 +686,7 @@ impl Harness {
self.compare_contents_recursive(fs_dir, reply.ino, node).await;
}
assert!(
keys.remove(name),
unvisited_ref_children.remove(name),
"file name must be in the set of child names in the reference fs",
);
}
Expand All @@ -684,19 +698,19 @@ impl Harness {
}

assert!(
keys.is_empty(),
"reference contained elements not in the filesystem in dir inode {fs_dir}: {keys:?}"
unvisited_ref_children.is_empty(),
"reference contained elements not in the filesystem in dir inode {fs_dir}: {unvisited_ref_children:?}"
);

self.fs.releasedir(fs_dir, dir_handle, 0).await.unwrap();
}
.boxed()
}

async fn compare_file<'a>(&'a self, fs_file: InodeNo, ref_file: &'a MockObject) {
let fh = match self.fs.open(fs_file, OpenFlags::empty(), 0).await {
async fn compare_file<'a>(&'a self, fs_ino: InodeNo, ref_file: &'a MockObject) {
let fh = match self.fs.open(fs_ino, OpenFlags::empty(), 0).await {
Ok(ret) => ret.fh,
Err(e) => panic!("failed to open ino {fs_file} for content comparison: {e:?}"),
Err(e) => panic!("failed to open ino {fs_ino} for content comparison: {e:?}"),
};
let mut offset = 0;
const MAX_READ_SIZE: usize = 128 * 1024;
Expand All @@ -707,7 +721,7 @@ impl Harness {
assert_eq!(ref_bytes.len(), num_bytes, "number of bytes did not match");
let bytes_from_read = self
.fs
.read(fs_file, fh, offset as i64, num_bytes as u32, 0, None)
.read(fs_ino, fh, offset as i64, num_bytes as u32, 0, None)
.await
.expect("read should succeed");
assert_eq!(&ref_bytes[..], &bytes_from_read, "read bytes did not match");
Expand Down
26 changes: 13 additions & 13 deletions mountpoint-s3/tests/reftests/reference.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Node {
#[derive(Debug)]
pub struct Reference {
/// Contents of our S3 bucket
remote_keys: BTreeMap<String, MockObject>,
remote_objects: BTreeMap<String, MockObject>,
/// Local files
local_files: Vec<PathBuf>,
/// Local directories
Expand Down Expand Up @@ -98,25 +98,25 @@ impl MaterializedReference {
);

let mut parent_node = &mut self.root;
while let Some(dir) = components.next() {
while let Some(name) = components.next() {
let Node::Directory { children, .. } = parent_node else {
return false;
// TODO: see above -- implicit directories are allowed to disappear
// panic!("unexpected internal file node while adding {:?}", path.as_ref());
};
let dir = dir.as_os_str().to_str().unwrap();
let name = name.as_os_str().to_str().unwrap();
if components.peek().is_none() {
// If both a local and a remote directory exist, don't overwrite the remote one's
// contents, as they will be visible even though the directory is local. But
// remember the directory is still local.
if typ == NodeType::Directory {
if let Some(Node::Directory { is_local, .. }) = children.get_mut(dir) {
if let Some(Node::Directory { is_local, .. }) = children.get_mut(name) {
*is_local = true;
break;
}
}
// If a directory of this name exists, ignore any local file
if let Some(node) = children.get(dir) {
if let Some(node) = children.get(name) {
if node.node_type() == NodeType::Directory {
return false;
}
Expand All @@ -128,15 +128,15 @@ impl MaterializedReference {
},
NodeType::File => Node::File(File::Local),
};
children.insert(dir.to_owned(), new_node);
children.insert(name.to_owned(), new_node);
break;
} else {
// TODO: see above -- implicit directories are allowed to disappear
// parent_node = children.entry(dir.to_owned()).or_insert_with(|| Node::Directory {
// children: BTreeMap::new(),
// is_local: true,
// })
let Some(child_node) = children.get_mut(dir) else {
let Some(child_node) = children.get_mut(name) else {
return false;
};
parent_node = child_node;
Expand All @@ -153,7 +153,7 @@ impl Reference {
let local_directories = vec![];
let materialized = build_reference(remote_keys.iter().map(|(k, o): &(_, _)| (k, o)));
Self {
remote_keys: remote_keys.into_iter().collect(),
remote_objects: remote_keys.into_iter().collect(),
local_files,
local_directories,
materialized,
Expand All @@ -162,10 +162,10 @@ impl Reference {

fn rematerialize(&self) -> MaterializedReference {
tracing::debug!(
remote_keys=?self.remote_keys, local_files=?self.local_files, local_directories=?self.local_directories,
remote_keys=?self.remote_objects.keys(), local_files=?self.local_files, local_directories=?self.local_directories,
"rematerialize",
);
let mut materialized = build_reference(self.remote_keys.iter());
let mut materialized = build_reference(self.remote_objects.iter());
for local_dir in self.local_directories.iter() {
let added = materialized.add_local_node(local_dir, NodeType::Directory);
if added {
Expand Down Expand Up @@ -252,12 +252,12 @@ impl Reference {
}

pub fn add_remote_key(&mut self, key: &str, object: MockObject) {
self.remote_keys.insert(key.to_owned(), object);
self.remote_objects.insert(key.to_owned(), object);
self.materialized = self.rematerialize();
}

pub fn remove_remote_key(&mut self, key: &str) {
self.remote_keys.remove(key);
self.remote_objects.remove(key);
self.materialized = self.rematerialize();
}

Expand Down Expand Up @@ -301,7 +301,7 @@ impl Reference {

/// A list of objects in the bucket
pub fn remote_keys(&self) -> impl ExactSizeIterator<Item = &str> {
self.remote_keys.keys().map(|key| key.as_str())
self.remote_objects.keys().map(|key| key.as_str())
}
}

Expand Down

0 comments on commit 7a9bea3

Please sign in to comment.