* Unit testing questions for FileStore::_detect_fs
@ 2013-02-06 11:51 Loic Dachary
2013-02-06 16:47 ` Sage Weil
0 siblings, 1 reply; 5+ messages in thread
From: Loic Dachary @ 2013-02-06 11:51 UTC (permalink / raw)
To: Ceph Development
[-- Attachment #1: Type: text/plain, Size: 5587 bytes --]
Hi,
The patch below adds unit tests for FileStore::_detect_fs, but it needs to run
as root in order to mount the ext3, ext4 and btrfs file systems created for test purposes.
Is there a way to mount filesystems without root privileges ?
configure.ac does not detect the presence of commands such as
mkfs.ext4 or mkfs.btrfs. The patch below does not add
AC_CHECK_TOOL([BTRFS], [mkfs.btrfs], ...) because it has no use
except for unit testing. Instead, the TEST_F(StoreTest, _detect_fs)
function checks if /sbin/mkfs.btrfs is an executable. If it is not,
"SKIP btrfs because /sbin/mkfs.btrfs is not executable" is displayed
and nothing is done.
The drawback is that it is quite difficult to figure out what tools
must be installed to maximize test coverage.
How would you implement unit test tools detection ?
The error returned when ext4 is mounted without user_xattr is the same
as the error returned when ext4 is is mounted with user_xattr but
without the filestore_xattr_use_omap option : -ENOTSUP. From the point
of view of the unit tests, they cannot be distinguished, except by
parsing the messages sent to derr which show
Extended attributes don't appear to work.
or
limited size xattrs -- enable filestore_xattr_use_omap
Is parsing the output a good practice to assert success or failure during unit testing ?
Cheers
diff --git a/src/test/filestore/store_test.cc b/src/test/filestore/store_test.cc
index c98ffb0..7ba38e2 100644
--- a/src/test/filestore/store_test.cc
+++ b/src/test/filestore/store_test.cc
@@ -16,6 +16,7 @@
#include <string.h>
#include <iostream>
#include <time.h>
+#include <sys/mount.h>
#include "os/FileStore.h"
#include "include/Context.h"
#include "common/ceph_argparse.h"
@@ -38,6 +39,7 @@ public:
StoreTest() : store(0) {}
virtual void SetUp() {
+ ::system("rm -fr store_test_temp_dir store_test_temp_journal");
::mkdir("store_test_temp_dir", 0777);
ObjectStore *store_ = new FileStore(string("store_test_temp_dir"), string("store_test_temp_journal"));
store.reset(store_);
@@ -823,6 +825,95 @@ TEST_F(StoreTest, ColSplitTest3) {
}
#endif
+TEST_F(StoreTest, _detect_fs) {
+ if (getuid() != 0) {
+ cerr << "SKIP because it does not run as root" << std::endl;
+ return;
+ }
+
+ if (access("/dev/zero", R_OK) != 0) {
+ cerr << "SKIP because /dev/zero is not a readable file" << std::endl;
+ return;
+ }
+
+ const string mnt("/tmp/test_filestore");
+ ::mkdir(mnt.c_str(), 0755);
+ ::umount(mnt.c_str());
+ const string disk(mnt + ".disk");
+ ::unlink(disk.c_str());
+ ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
+
+ const string dir("store_test_temp_dir");
+ const string journal("store_test_temp_journal");
+
+ const string mkfs_ext4("/sbin/mkfs.ext4");
+ if (::access(mkfs_ext4.c_str(), X_OK) == 0) {
+
+ ASSERT_EQ(::system((mkfs_ext4 + " -q -F " + disk).c_str()), 0);
+ //
+ // without user_xattr, ext4 fails
+ //
+ {
+ g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
+ ASSERT_EQ(::system((string("mount -o loop,nouser_xattr ") + disk + " " + mnt).c_str()), 0);
+ ASSERT_EQ(::chdir(mnt.c_str()), 0);
+ ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
+ FileStore store(dir, journal);
+ ASSERT_EQ(store._detect_fs(), -ENOTSUP);
+ ASSERT_EQ(::chdir("/tmp"), 0);
+ ASSERT_EQ(::umount(mnt.c_str()), 0);
+ }
+ //
+ // mounted with user_xattr, ext4 fails if filestore_xattr_use_omap is false
+ //
+ {
+ g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
+ ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
+ ASSERT_EQ(::chdir(mnt.c_str()), 0);
+ FileStore store(dir, journal);
+ ASSERT_EQ(store._detect_fs(), -ENOTSUP);
+ ASSERT_EQ(::chdir("/tmp"), 0);
+ ASSERT_EQ(::umount(mnt.c_str()), 0);
+ }
+ //
+ // mounted with user_xattr, ext4 succeeds if filestore_xattr_use_omap is true
+ //
+ {
+ g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
+ ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
+ ASSERT_EQ(::chdir(mnt.c_str()), 0);
+ FileStore store(dir, journal);
+ ASSERT_EQ(store._detect_fs(), 0);
+ ASSERT_EQ(::chdir("/tmp"), 0);
+ ASSERT_EQ(::umount(mnt.c_str()), 0);
+ }
+ } else {
+ cerr << "SKIP ext4 because " << mkfs_ext4 << " is not executable" << std::endl;
+ }
+
+ g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
+ ASSERT_EQ(::unlink(disk.c_str()), 0);
+ ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
+ const string mkfs_btrfs("/sbin/mkfs.btrfs");
+ if (::access(mkfs_btrfs.c_str(), X_OK) == 0) {
+ ASSERT_EQ(::system((mkfs_btrfs + " " + disk).c_str()), 0);
+ //
+ // btrfs succeeds
+ //
+ {
+ ASSERT_EQ(::system((string("mount -o loop ") + disk + " " + mnt).c_str()), 0);
+ ASSERT_EQ(::chdir(mnt.c_str()), 0);
+ ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
+ FileStore store(dir, journal);
+ ASSERT_EQ(store._detect_fs(), 0);
+ ASSERT_EQ(::chdir("/tmp"), 0);
+ ASSERT_EQ(::umount(mnt.c_str()), 0);
+ }
+ } else {
+ cerr << "SKIP btrfs because " << mkfs_btrfs << " is not executable" << std::endl;
+ }
+}
+
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: Unit testing questions for FileStore::_detect_fs
2013-02-06 11:51 Unit testing questions for FileStore::_detect_fs Loic Dachary
@ 2013-02-06 16:47 ` Sage Weil
2013-02-10 10:36 ` Loic Dachary
0 siblings, 1 reply; 5+ messages in thread
From: Sage Weil @ 2013-02-06 16:47 UTC (permalink / raw)
To: Loic Dachary; +Cc: Ceph Development
On Wed, 6 Feb 2013, Loic Dachary wrote:
> Hi,
>
> The patch below adds unit tests for FileStore::_detect_fs, but it needs to run
> as root in order to mount the ext3, ext4 and btrfs file systems created for test purposes.
>
> Is there a way to mount filesystems without root privileges ?
Not that I know of.
There are several other tests that are meant ot be run separate from the
build environment that use gtest (test_filejournal, the api functional
tests, etc.). As long as this is built to a separate binary, I don't
think it's a problem. We should converge on unittest_* for the stuff
'make check' runs and test_* for stuff that is meant to run elsewhere.
Having our normal qa harness be root is not a problem.
> configure.ac does not detect the presence of commands such as
> mkfs.ext4 or mkfs.btrfs. The patch below does not add
> AC_CHECK_TOOL([BTRFS], [mkfs.btrfs], ...) because it has no use
> except for unit testing. Instead, the TEST_F(StoreTest, _detect_fs)
> function checks if /sbin/mkfs.btrfs is an executable. If it is not,
> "SKIP btrfs because /sbin/mkfs.btrfs is not executable" is displayed
> and nothing is done.
>
> The drawback is that it is quite difficult to figure out what tools
> must be installed to maximize test coverage.
>
> How would you implement unit test tools detection ?
>
> The error returned when ext4 is mounted without user_xattr is the same
> as the error returned when ext4 is is mounted with user_xattr but
> without the filestore_xattr_use_omap option : -ENOTSUP. From the point
> of view of the unit tests, they cannot be distinguished, except by
> parsing the messages sent to derr which show
>
> Extended attributes don't appear to work.
>
> or
>
> limited size xattrs -- enable filestore_xattr_use_omap
>
> Is parsing the output a good practice to assert success or failure during unit testing ?
I'm not sure it is worth the complexity. IMO it would be better to change
the return value (suggestions?) or not both distinguishing between the two
cases in the test.
sage
>
> Cheers
>
> diff --git a/src/test/filestore/store_test.cc b/src/test/filestore/store_test.cc
> index c98ffb0..7ba38e2 100644
> --- a/src/test/filestore/store_test.cc
> +++ b/src/test/filestore/store_test.cc
> @@ -16,6 +16,7 @@
> #include <string.h>
> #include <iostream>
> #include <time.h>
> +#include <sys/mount.h>
> #include "os/FileStore.h"
> #include "include/Context.h"
> #include "common/ceph_argparse.h"
> @@ -38,6 +39,7 @@ public:
>
> StoreTest() : store(0) {}
> virtual void SetUp() {
> + ::system("rm -fr store_test_temp_dir store_test_temp_journal");
> ::mkdir("store_test_temp_dir", 0777);
> ObjectStore *store_ = new FileStore(string("store_test_temp_dir"), string("store_test_temp_journal"));
> store.reset(store_);
> @@ -823,6 +825,95 @@ TEST_F(StoreTest, ColSplitTest3) {
> }
> #endif
>
> +TEST_F(StoreTest, _detect_fs) {
> + if (getuid() != 0) {
> + cerr << "SKIP because it does not run as root" << std::endl;
> + return;
> + }
> +
> + if (access("/dev/zero", R_OK) != 0) {
> + cerr << "SKIP because /dev/zero is not a readable file" << std::endl;
> + return;
> + }
> +
> + const string mnt("/tmp/test_filestore");
> + ::mkdir(mnt.c_str(), 0755);
> + ::umount(mnt.c_str());
> + const string disk(mnt + ".disk");
> + ::unlink(disk.c_str());
> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
> +
> + const string dir("store_test_temp_dir");
> + const string journal("store_test_temp_journal");
> +
> + const string mkfs_ext4("/sbin/mkfs.ext4");
> + if (::access(mkfs_ext4.c_str(), X_OK) == 0) {
> +
> + ASSERT_EQ(::system((mkfs_ext4 + " -q -F " + disk).c_str()), 0);
> + //
> + // without user_xattr, ext4 fails
> + //
> + {
> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
> + ASSERT_EQ(::system((string("mount -o loop,nouser_xattr ") + disk + " " + mnt).c_str()), 0);
> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
> + FileStore store(dir, journal);
> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
> + ASSERT_EQ(::chdir("/tmp"), 0);
> + ASSERT_EQ(::umount(mnt.c_str()), 0);
> + }
> + //
> + // mounted with user_xattr, ext4 fails if filestore_xattr_use_omap is false
> + //
> + {
> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
> + FileStore store(dir, journal);
> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
> + ASSERT_EQ(::chdir("/tmp"), 0);
> + ASSERT_EQ(::umount(mnt.c_str()), 0);
> + }
> + //
> + // mounted with user_xattr, ext4 succeeds if filestore_xattr_use_omap is true
> + //
> + {
> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
> + FileStore store(dir, journal);
> + ASSERT_EQ(store._detect_fs(), 0);
> + ASSERT_EQ(::chdir("/tmp"), 0);
> + ASSERT_EQ(::umount(mnt.c_str()), 0);
> + }
> + } else {
> + cerr << "SKIP ext4 because " << mkfs_ext4 << " is not executable" << std::endl;
> + }
> +
> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
> + ASSERT_EQ(::unlink(disk.c_str()), 0);
> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
> + const string mkfs_btrfs("/sbin/mkfs.btrfs");
> + if (::access(mkfs_btrfs.c_str(), X_OK) == 0) {
> + ASSERT_EQ(::system((mkfs_btrfs + " " + disk).c_str()), 0);
> + //
> + // btrfs succeeds
> + //
> + {
> + ASSERT_EQ(::system((string("mount -o loop ") + disk + " " + mnt).c_str()), 0);
> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
> + FileStore store(dir, journal);
> + ASSERT_EQ(store._detect_fs(), 0);
> + ASSERT_EQ(::chdir("/tmp"), 0);
> + ASSERT_EQ(::umount(mnt.c_str()), 0);
> + }
> + } else {
> + cerr << "SKIP btrfs because " << mkfs_btrfs << " is not executable" << std::endl;
> + }
> +}
> +
>
> --
> Lo?c Dachary, Artisan Logiciel Libre
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Unit testing questions for FileStore::_detect_fs
2013-02-06 16:47 ` Sage Weil
@ 2013-02-10 10:36 ` Loic Dachary
2013-02-16 16:33 ` Sam Lang
0 siblings, 1 reply; 5+ messages in thread
From: Loic Dachary @ 2013-02-10 10:36 UTC (permalink / raw)
To: Sage Weil; +Cc: Ceph Development
[-- Attachment #1: Type: text/plain, Size: 7081 bytes --]
Hi,
On 02/06/2013 05:47 PM, Sage Weil wrote:
> On Wed, 6 Feb 2013, Loic Dachary wrote:
>> Hi,
>>
>> The patch below adds unit tests for FileStore::_detect_fs, but it needs to run
>> as root in order to mount the ext3, ext4 and btrfs file systems created for test purposes.
>>
>> Is there a way to mount filesystems without root privileges ?
>
> Not that I know of.
>
> There are several other tests that are meant ot be run separate from the
> build environment that use gtest (test_filejournal, the api functional
> tests, etc.). As long as this is built to a separate binary, I don't
> think it's a problem. We should converge on unittest_* for the stuff
> 'make check' runs and test_* for stuff that is meant to run elsewhere.
> Having our normal qa harness be root is not a problem.
If I understand correctly, these test_* binaries can then be included in scripts found in ceph/qa/workunits and used by teuthology. The simplest example being https://github.com/ceph/ceph/blob/master/qa/workunits/rbd/test_librbd.sh
Cheers
>> configure.ac does not detect the presence of commands such as
>> mkfs.ext4 or mkfs.btrfs. The patch below does not add
>> AC_CHECK_TOOL([BTRFS], [mkfs.btrfs], ...) because it has no use
>> except for unit testing. Instead, the TEST_F(StoreTest, _detect_fs)
>> function checks if /sbin/mkfs.btrfs is an executable. If it is not,
>> "SKIP btrfs because /sbin/mkfs.btrfs is not executable" is displayed
>> and nothing is done.
>>
>> The drawback is that it is quite difficult to figure out what tools
>> must be installed to maximize test coverage.
>>
>> How would you implement unit test tools detection ?
>>
>> The error returned when ext4 is mounted without user_xattr is the same
>> as the error returned when ext4 is is mounted with user_xattr but
>> without the filestore_xattr_use_omap option : -ENOTSUP. From the point
>> of view of the unit tests, they cannot be distinguished, except by
>> parsing the messages sent to derr which show
>>
>> Extended attributes don't appear to work.
>>
>> or
>>
>> limited size xattrs -- enable filestore_xattr_use_omap
>>
>> Is parsing the output a good practice to assert success or failure during unit testing ?
>
> I'm not sure it is worth the complexity. IMO it would be better to change
> the return value (suggestions?) or not both distinguishing between the two
> cases in the test.
>
> sage
>
>>
>> Cheers
>>
>> diff --git a/src/test/filestore/store_test.cc b/src/test/filestore/store_test.cc
>> index c98ffb0..7ba38e2 100644
>> --- a/src/test/filestore/store_test.cc
>> +++ b/src/test/filestore/store_test.cc
>> @@ -16,6 +16,7 @@
>> #include <string.h>
>> #include <iostream>
>> #include <time.h>
>> +#include <sys/mount.h>
>> #include "os/FileStore.h"
>> #include "include/Context.h"
>> #include "common/ceph_argparse.h"
>> @@ -38,6 +39,7 @@ public:
>>
>> StoreTest() : store(0) {}
>> virtual void SetUp() {
>> + ::system("rm -fr store_test_temp_dir store_test_temp_journal");
>> ::mkdir("store_test_temp_dir", 0777);
>> ObjectStore *store_ = new FileStore(string("store_test_temp_dir"), string("store_test_temp_journal"));
>> store.reset(store_);
>> @@ -823,6 +825,95 @@ TEST_F(StoreTest, ColSplitTest3) {
>> }
>> #endif
>>
>> +TEST_F(StoreTest, _detect_fs) {
>> + if (getuid() != 0) {
>> + cerr << "SKIP because it does not run as root" << std::endl;
>> + return;
>> + }
>> +
>> + if (access("/dev/zero", R_OK) != 0) {
>> + cerr << "SKIP because /dev/zero is not a readable file" << std::endl;
>> + return;
>> + }
>> +
>> + const string mnt("/tmp/test_filestore");
>> + ::mkdir(mnt.c_str(), 0755);
>> + ::umount(mnt.c_str());
>> + const string disk(mnt + ".disk");
>> + ::unlink(disk.c_str());
>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>> +
>> + const string dir("store_test_temp_dir");
>> + const string journal("store_test_temp_journal");
>> +
>> + const string mkfs_ext4("/sbin/mkfs.ext4");
>> + if (::access(mkfs_ext4.c_str(), X_OK) == 0) {
>> +
>> + ASSERT_EQ(::system((mkfs_ext4 + " -q -F " + disk).c_str()), 0);
>> + //
>> + // without user_xattr, ext4 fails
>> + //
>> + {
>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>> + ASSERT_EQ(::system((string("mount -o loop,nouser_xattr ") + disk + " " + mnt).c_str()), 0);
>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>> + FileStore store(dir, journal);
>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>> + ASSERT_EQ(::chdir("/tmp"), 0);
>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>> + }
>> + //
>> + // mounted with user_xattr, ext4 fails if filestore_xattr_use_omap is false
>> + //
>> + {
>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>> + FileStore store(dir, journal);
>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>> + ASSERT_EQ(::chdir("/tmp"), 0);
>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>> + }
>> + //
>> + // mounted with user_xattr, ext4 succeeds if filestore_xattr_use_omap is true
>> + //
>> + {
>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>> + FileStore store(dir, journal);
>> + ASSERT_EQ(store._detect_fs(), 0);
>> + ASSERT_EQ(::chdir("/tmp"), 0);
>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>> + }
>> + } else {
>> + cerr << "SKIP ext4 because " << mkfs_ext4 << " is not executable" << std::endl;
>> + }
>> +
>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>> + ASSERT_EQ(::unlink(disk.c_str()), 0);
>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>> + const string mkfs_btrfs("/sbin/mkfs.btrfs");
>> + if (::access(mkfs_btrfs.c_str(), X_OK) == 0) {
>> + ASSERT_EQ(::system((mkfs_btrfs + " " + disk).c_str()), 0);
>> + //
>> + // btrfs succeeds
>> + //
>> + {
>> + ASSERT_EQ(::system((string("mount -o loop ") + disk + " " + mnt).c_str()), 0);
>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>> + FileStore store(dir, journal);
>> + ASSERT_EQ(store._detect_fs(), 0);
>> + ASSERT_EQ(::chdir("/tmp"), 0);
>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>> + }
>> + } else {
>> + cerr << "SKIP btrfs because " << mkfs_btrfs << " is not executable" << std::endl;
>> + }
>> +}
>> +
>>
>> --
>> Lo?c Dachary, Artisan Logiciel Libre
>>
>>
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Unit testing questions for FileStore::_detect_fs
2013-02-10 10:36 ` Loic Dachary
@ 2013-02-16 16:33 ` Sam Lang
2013-02-16 18:54 ` Loic Dachary
0 siblings, 1 reply; 5+ messages in thread
From: Sam Lang @ 2013-02-16 16:33 UTC (permalink / raw)
To: Loic Dachary; +Cc: Sage Weil, Ceph Development
On Sun, Feb 10, 2013 at 4:36 AM, Loic Dachary <loic@dachary.org> wrote:
> Hi,
>
> On 02/06/2013 05:47 PM, Sage Weil wrote:
>> On Wed, 6 Feb 2013, Loic Dachary wrote:
>>> Hi,
>>>
>>> The patch below adds unit tests for FileStore::_detect_fs, but it needs to run
>>> as root in order to mount the ext3, ext4 and btrfs file systems created for test purposes.
>>>
>>> Is there a way to mount filesystems without root privileges ?
>>
>> Not that I know of.
>>
>> There are several other tests that are meant ot be run separate from the
>> build environment that use gtest (test_filejournal, the api functional
>> tests, etc.). As long as this is built to a separate binary, I don't
>> think it's a problem. We should converge on unittest_* for the stuff
>> 'make check' runs and test_* for stuff that is meant to run elsewhere.
>> Having our normal qa harness be root is not a problem.
>
> If I understand correctly, these test_* binaries can then be included in scripts found in ceph/qa/workunits and used by teuthology. The simplest example being https://github.com/ceph/ceph/blob/master/qa/workunits/rbd/test_librbd.sh
Yep that's right. Note that the test_* binaries were recently renamed
to ceph_test_*
-sam
>
> Cheers
>
>>> configure.ac does not detect the presence of commands such as
>>> mkfs.ext4 or mkfs.btrfs. The patch below does not add
>>> AC_CHECK_TOOL([BTRFS], [mkfs.btrfs], ...) because it has no use
>>> except for unit testing. Instead, the TEST_F(StoreTest, _detect_fs)
>>> function checks if /sbin/mkfs.btrfs is an executable. If it is not,
>>> "SKIP btrfs because /sbin/mkfs.btrfs is not executable" is displayed
>>> and nothing is done.
>>>
>>> The drawback is that it is quite difficult to figure out what tools
>>> must be installed to maximize test coverage.
>>>
>>> How would you implement unit test tools detection ?
>>>
>>> The error returned when ext4 is mounted without user_xattr is the same
>>> as the error returned when ext4 is is mounted with user_xattr but
>>> without the filestore_xattr_use_omap option : -ENOTSUP. From the point
>>> of view of the unit tests, they cannot be distinguished, except by
>>> parsing the messages sent to derr which show
>>>
>>> Extended attributes don't appear to work.
>>>
>>> or
>>>
>>> limited size xattrs -- enable filestore_xattr_use_omap
>>>
>>> Is parsing the output a good practice to assert success or failure during unit testing ?
>>
>> I'm not sure it is worth the complexity. IMO it would be better to change
>> the return value (suggestions?) or not both distinguishing between the two
>> cases in the test.
>>
>> sage
>>
>>>
>>> Cheers
>>>
>>> diff --git a/src/test/filestore/store_test.cc b/src/test/filestore/store_test.cc
>>> index c98ffb0..7ba38e2 100644
>>> --- a/src/test/filestore/store_test.cc
>>> +++ b/src/test/filestore/store_test.cc
>>> @@ -16,6 +16,7 @@
>>> #include <string.h>
>>> #include <iostream>
>>> #include <time.h>
>>> +#include <sys/mount.h>
>>> #include "os/FileStore.h"
>>> #include "include/Context.h"
>>> #include "common/ceph_argparse.h"
>>> @@ -38,6 +39,7 @@ public:
>>>
>>> StoreTest() : store(0) {}
>>> virtual void SetUp() {
>>> + ::system("rm -fr store_test_temp_dir store_test_temp_journal");
>>> ::mkdir("store_test_temp_dir", 0777);
>>> ObjectStore *store_ = new FileStore(string("store_test_temp_dir"), string("store_test_temp_journal"));
>>> store.reset(store_);
>>> @@ -823,6 +825,95 @@ TEST_F(StoreTest, ColSplitTest3) {
>>> }
>>> #endif
>>>
>>> +TEST_F(StoreTest, _detect_fs) {
>>> + if (getuid() != 0) {
>>> + cerr << "SKIP because it does not run as root" << std::endl;
>>> + return;
>>> + }
>>> +
>>> + if (access("/dev/zero", R_OK) != 0) {
>>> + cerr << "SKIP because /dev/zero is not a readable file" << std::endl;
>>> + return;
>>> + }
>>> +
>>> + const string mnt("/tmp/test_filestore");
>>> + ::mkdir(mnt.c_str(), 0755);
>>> + ::umount(mnt.c_str());
>>> + const string disk(mnt + ".disk");
>>> + ::unlink(disk.c_str());
>>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>>> +
>>> + const string dir("store_test_temp_dir");
>>> + const string journal("store_test_temp_journal");
>>> +
>>> + const string mkfs_ext4("/sbin/mkfs.ext4");
>>> + if (::access(mkfs_ext4.c_str(), X_OK) == 0) {
>>> +
>>> + ASSERT_EQ(::system((mkfs_ext4 + " -q -F " + disk).c_str()), 0);
>>> + //
>>> + // without user_xattr, ext4 fails
>>> + //
>>> + {
>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>>> + ASSERT_EQ(::system((string("mount -o loop,nouser_xattr ") + disk + " " + mnt).c_str()), 0);
>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>>> + FileStore store(dir, journal);
>>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>> + }
>>> + //
>>> + // mounted with user_xattr, ext4 fails if filestore_xattr_use_omap is false
>>> + //
>>> + {
>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>> + FileStore store(dir, journal);
>>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>> + }
>>> + //
>>> + // mounted with user_xattr, ext4 succeeds if filestore_xattr_use_omap is true
>>> + //
>>> + {
>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>> + FileStore store(dir, journal);
>>> + ASSERT_EQ(store._detect_fs(), 0);
>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>> + }
>>> + } else {
>>> + cerr << "SKIP ext4 because " << mkfs_ext4 << " is not executable" << std::endl;
>>> + }
>>> +
>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>>> + ASSERT_EQ(::unlink(disk.c_str()), 0);
>>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>>> + const string mkfs_btrfs("/sbin/mkfs.btrfs");
>>> + if (::access(mkfs_btrfs.c_str(), X_OK) == 0) {
>>> + ASSERT_EQ(::system((mkfs_btrfs + " " + disk).c_str()), 0);
>>> + //
>>> + // btrfs succeeds
>>> + //
>>> + {
>>> + ASSERT_EQ(::system((string("mount -o loop ") + disk + " " + mnt).c_str()), 0);
>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>>> + FileStore store(dir, journal);
>>> + ASSERT_EQ(store._detect_fs(), 0);
>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>> + }
>>> + } else {
>>> + cerr << "SKIP btrfs because " << mkfs_btrfs << " is not executable" << std::endl;
>>> + }
>>> +}
>>> +
>>>
>>> --
>>> Lo?c Dachary, Artisan Logiciel Libre
>>>
>>>
>
> --
> Loïc Dachary, Artisan Logiciel Libre
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Unit testing questions for FileStore::_detect_fs
2013-02-16 16:33 ` Sam Lang
@ 2013-02-16 18:54 ` Loic Dachary
0 siblings, 0 replies; 5+ messages in thread
From: Loic Dachary @ 2013-02-16 18:54 UTC (permalink / raw)
To: Sam Lang; +Cc: Ceph Development
[-- Attachment #1: Type: text/plain, Size: 8266 bytes --]
On 02/16/2013 05:33 PM, Sam Lang wrote:
> On Sun, Feb 10, 2013 at 4:36 AM, Loic Dachary <loic@dachary.org> wrote:
>> Hi,
>>
>> On 02/06/2013 05:47 PM, Sage Weil wrote:
>>> On Wed, 6 Feb 2013, Loic Dachary wrote:
>>>> Hi,
>>>>
>>>> The patch below adds unit tests for FileStore::_detect_fs, but it needs to run
>>>> as root in order to mount the ext3, ext4 and btrfs file systems created for test purposes.
>>>>
>>>> Is there a way to mount filesystems without root privileges ?
>>>
>>> Not that I know of.
>>>
>>> There are several other tests that are meant ot be run separate from the
>>> build environment that use gtest (test_filejournal, the api functional
>>> tests, etc.). As long as this is built to a separate binary, I don't
>>> think it's a problem. We should converge on unittest_* for the stuff
>>> 'make check' runs and test_* for stuff that is meant to run elsewhere.
>>> Having our normal qa harness be root is not a problem.
>>
>> If I understand correctly, these test_* binaries can then be included in scripts found in ceph/qa/workunits and used by teuthology. The simplest example being https://github.com/ceph/ceph/blob/master/qa/workunits/rbd/test_librbd.sh
>
> Yep that's right. Note that the test_* binaries were recently renamed
> to ceph_test_*
Thanks for the hint. Now that I'm done with unit testing buffer.{h,cc} ( https://github.com/dachary/ceph/commit/9e22d60eda488195337ed533448c679bf8d43639 ) I was about to work on improving ceph_test_filestore and run it on various file systems with teuthology. Let me know if my time would be better used working on something else ;-)
Cheers
> -sam
>
>>
>> Cheers
>>
>>>> configure.ac does not detect the presence of commands such as
>>>> mkfs.ext4 or mkfs.btrfs. The patch below does not add
>>>> AC_CHECK_TOOL([BTRFS], [mkfs.btrfs], ...) because it has no use
>>>> except for unit testing. Instead, the TEST_F(StoreTest, _detect_fs)
>>>> function checks if /sbin/mkfs.btrfs is an executable. If it is not,
>>>> "SKIP btrfs because /sbin/mkfs.btrfs is not executable" is displayed
>>>> and nothing is done.
>>>>
>>>> The drawback is that it is quite difficult to figure out what tools
>>>> must be installed to maximize test coverage.
>>>>
>>>> How would you implement unit test tools detection ?
>>>>
>>>> The error returned when ext4 is mounted without user_xattr is the same
>>>> as the error returned when ext4 is is mounted with user_xattr but
>>>> without the filestore_xattr_use_omap option : -ENOTSUP. From the point
>>>> of view of the unit tests, they cannot be distinguished, except by
>>>> parsing the messages sent to derr which show
>>>>
>>>> Extended attributes don't appear to work.
>>>>
>>>> or
>>>>
>>>> limited size xattrs -- enable filestore_xattr_use_omap
>>>>
>>>> Is parsing the output a good practice to assert success or failure during unit testing ?
>>>
>>> I'm not sure it is worth the complexity. IMO it would be better to change
>>> the return value (suggestions?) or not both distinguishing between the two
>>> cases in the test.
>>>
>>> sage
>>>
>>>>
>>>> Cheers
>>>>
>>>> diff --git a/src/test/filestore/store_test.cc b/src/test/filestore/store_test.cc
>>>> index c98ffb0..7ba38e2 100644
>>>> --- a/src/test/filestore/store_test.cc
>>>> +++ b/src/test/filestore/store_test.cc
>>>> @@ -16,6 +16,7 @@
>>>> #include <string.h>
>>>> #include <iostream>
>>>> #include <time.h>
>>>> +#include <sys/mount.h>
>>>> #include "os/FileStore.h"
>>>> #include "include/Context.h"
>>>> #include "common/ceph_argparse.h"
>>>> @@ -38,6 +39,7 @@ public:
>>>>
>>>> StoreTest() : store(0) {}
>>>> virtual void SetUp() {
>>>> + ::system("rm -fr store_test_temp_dir store_test_temp_journal");
>>>> ::mkdir("store_test_temp_dir", 0777);
>>>> ObjectStore *store_ = new FileStore(string("store_test_temp_dir"), string("store_test_temp_journal"));
>>>> store.reset(store_);
>>>> @@ -823,6 +825,95 @@ TEST_F(StoreTest, ColSplitTest3) {
>>>> }
>>>> #endif
>>>>
>>>> +TEST_F(StoreTest, _detect_fs) {
>>>> + if (getuid() != 0) {
>>>> + cerr << "SKIP because it does not run as root" << std::endl;
>>>> + return;
>>>> + }
>>>> +
>>>> + if (access("/dev/zero", R_OK) != 0) {
>>>> + cerr << "SKIP because /dev/zero is not a readable file" << std::endl;
>>>> + return;
>>>> + }
>>>> +
>>>> + const string mnt("/tmp/test_filestore");
>>>> + ::mkdir(mnt.c_str(), 0755);
>>>> + ::umount(mnt.c_str());
>>>> + const string disk(mnt + ".disk");
>>>> + ::unlink(disk.c_str());
>>>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>>>> +
>>>> + const string dir("store_test_temp_dir");
>>>> + const string journal("store_test_temp_journal");
>>>> +
>>>> + const string mkfs_ext4("/sbin/mkfs.ext4");
>>>> + if (::access(mkfs_ext4.c_str(), X_OK) == 0) {
>>>> +
>>>> + ASSERT_EQ(::system((mkfs_ext4 + " -q -F " + disk).c_str()), 0);
>>>> + //
>>>> + // without user_xattr, ext4 fails
>>>> + //
>>>> + {
>>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>>>> + ASSERT_EQ(::system((string("mount -o loop,nouser_xattr ") + disk + " " + mnt).c_str()), 0);
>>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>>>> + FileStore store(dir, journal);
>>>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>>> + }
>>>> + //
>>>> + // mounted with user_xattr, ext4 fails if filestore_xattr_use_omap is false
>>>> + //
>>>> + {
>>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>>>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>>> + FileStore store(dir, journal);
>>>> + ASSERT_EQ(store._detect_fs(), -ENOTSUP);
>>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>>> + }
>>>> + //
>>>> + // mounted with user_xattr, ext4 succeeds if filestore_xattr_use_omap is true
>>>> + //
>>>> + {
>>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "true");
>>>> + ASSERT_EQ(::system((string("mount -o loop,user_xattr ") + disk + " " + mnt).c_str()), 0);
>>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>>> + FileStore store(dir, journal);
>>>> + ASSERT_EQ(store._detect_fs(), 0);
>>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>>> + }
>>>> + } else {
>>>> + cerr << "SKIP ext4 because " << mkfs_ext4 << " is not executable" << std::endl;
>>>> + }
>>>> +
>>>> + g_ceph_context->_conf->set_val("filestore_xattr_use_omap", "false");
>>>> + ASSERT_EQ(::unlink(disk.c_str()), 0);
>>>> + ASSERT_EQ(::system(string("dd if=/dev/zero of=" + disk + " bs=1 count=0 seek=1G").c_str()), 0);
>>>> + const string mkfs_btrfs("/sbin/mkfs.btrfs");
>>>> + if (::access(mkfs_btrfs.c_str(), X_OK) == 0) {
>>>> + ASSERT_EQ(::system((mkfs_btrfs + " " + disk).c_str()), 0);
>>>> + //
>>>> + // btrfs succeeds
>>>> + //
>>>> + {
>>>> + ASSERT_EQ(::system((string("mount -o loop ") + disk + " " + mnt).c_str()), 0);
>>>> + ASSERT_EQ(::chdir(mnt.c_str()), 0);
>>>> + ASSERT_EQ(::mkdir(dir.c_str(), 0755), 0);
>>>> + FileStore store(dir, journal);
>>>> + ASSERT_EQ(store._detect_fs(), 0);
>>>> + ASSERT_EQ(::chdir("/tmp"), 0);
>>>> + ASSERT_EQ(::umount(mnt.c_str()), 0);
>>>> + }
>>>> + } else {
>>>> + cerr << "SKIP btrfs because " << mkfs_btrfs << " is not executable" << std::endl;
>>>> + }
>>>> +}
>>>> +
>>>>
>>>> --
>>>> Lo?c Dachary, Artisan Logiciel Libre
>>>>
>>>>
>>
>> --
>> Loïc Dachary, Artisan Logiciel Libre
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Loïc Dachary, Artisan Logiciel Libre
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-16 18:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-06 11:51 Unit testing questions for FileStore::_detect_fs Loic Dachary
2013-02-06 16:47 ` Sage Weil
2013-02-10 10:36 ` Loic Dachary
2013-02-16 16:33 ` Sam Lang
2013-02-16 18:54 ` Loic Dachary
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.