function should do the require for everything it needs that may not be available. It's doing for the inspect-internal command, but it's missing a: _require_btrfs_sysfs_fsid
Yes, it did. Actually, check_fsid() would need the following to cover all the prerequisites. _require_btrfs_fs_sysfs _require_btrfs_fs_feature temp_fsid _require_btrfs_fs_feature metadata_uuid _require_btrfs_command inspect-internal dump-super I already have v4 with what you just suggested, I am going to send it. > Instead this is being called for every test case that calls this new > helper function, when those requirements should be hidden from the > tests themselves. However, I am a bit skeptical if we should move all prerequisites to the helpers or only some major prerequisites. Because returning _notrun() in the middle of the testcase is something I am not sure is better than at the beginning of the testcase (I do not have a specific example where it is not a good idea, though). And, theoretically, figuring out if the test case would run/_notrun() will be complicated. Next, we shall end up checking the _require..() multiple times in a test case, though one time is enough (the test cases 311, 312, 313 call check_fsid() two times). Furthermore, it will inconsistent, as a lot of command wraps are already missing such a requirement; I'm not sure if we shall ever achieve consistency across fstests (For example: _cp_reflink() missing _require_cp_reflink). Lastly, if there are duplicating prerequisites across the helper functions, then we call _require..() many more times (for example: 313 will call mkfs_clone() and check_fsid() two times, which means we would verify the following three times in a testcase. _require_btrfs_fs_feature metadata_uuid _require_btrfs_command inspect-internal dump-super So, how about prerequisites of the newer functions as comments above the function to be copied into the test case? Thanks, Anand
