Allow setting DIO_TEST_DEV during runtime, by reading the environment
variable. The test was fragile despite the delay, because the real
io_getevents() call isn't guaranteed to return the number of events
requested. Fix that. Moreover, allow reading DIO_TEST_DELAY (in us)
from the environment. With the io_getevents fix, for me the test
succeeded even with 0 us delay.

Change the README accordingly.

Signed-off-by: Martin Wilck <[email protected]>
---
 tests/Makefile   |   8 ---
 tests/README.md  |  29 ++++++--
 tests/directio.c | 182 ++++++++++++++++++++++++++---------------------
 3 files changed, 122 insertions(+), 97 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 55fbf0f..02580e7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -21,12 +21,6 @@ valgrind:    $(TESTS:%=%.vgr)
 # test-specific compiler flags
 # XYZ-test_FLAGS: Additional compiler flags for this test
 
-ifneq ($(wildcard directio_test_dev),)
-DIO_TEST_DEV = $(shell sed -n -e 
's/^[[:space:]]*DIO_TEST_DEV[[:space:]]*=[[:space:]]*\([^[:space:]\#]\+\).*/\1/p'
 < directio_test_dev)
-endif
-ifneq ($(DIO_TEST_DEV),)
-directio-test_FLAGS := -DDIO_TEST_DEV=\"$(DIO_TEST_DEV)\"
-endif
 mpathvalid-test_FLAGS := -I$(mpathvaliddir)
 features-test_FLAGS := -I$(multipathdir)/nvme
 
@@ -59,9 +53,7 @@ valid-test_LIBDEPS := -lmount -ludev -lpthread -ldl
 devt-test_LIBDEPS := -ludev
 mpathvalid-test_LIBDEPS := -ludev -lpthread -ldl
 mpathvalid-test_OBJDEPS := $(mpathvaliddir)/mpath_valid.o
-ifneq ($(DIO_TEST_DEV),)
 directio-test_LIBDEPS := -laio
-endif
 strbuf-test_OBJDEPS := $(mpathutildir)/strbuf.o
 sysfs-test_TESTDEPS := test-log.o
 sysfs-test_OBJDEPS := $(multipathdir)/sysfs.o $(mpathutildir)/util.o
diff --git a/tests/README.md b/tests/README.md
index fd36fc1..0cae057 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -13,6 +13,17 @@ If valgrind detects a bad memory access or leak, the test 
will fail. The
 output of the test run, including valgrind output, is stored as
 `<testname>.vgr`.
 
+## Running tests manually
+
+`make test` or `make -C test "$TEST.out"` will only run the test program if
+the output files `$TEST.out` don't exist yet. To re-run the test, delete the
+output file first. In order to run a test outside `make`, set the library
+search path:
+
+    cd tests
+    export LD_LIBRARY_PATH=.:../libmpathutil:../libmpathcmd
+       ./dmevents-test  # or whatever other test you want to run
+
 ## Controlling verbosity for unit tests
 
 Some test programs use the environment variable `MPATHTEST_VERBOSITY` to
@@ -37,15 +48,21 @@ This test includes test items that require a access to a 
block device. The
 device will be opened in read-only mode; you don't need to worry about data
 loss. However, the user needs to specify a device to be used. Set the
 environment variable `DIO_TEST_DEV` to the path of the device.
-Alternatively, create a file `directio_test_dev` under
-the `tests` directory containing a single line that sets this environment
-variable in Bourne Shell syntax, like this:
-
-    DIO_TEST_DEV=/dev/sdc3
-
 After that, run `make directio.out` as root in the `tests` directory to
 perform the test.
 
+With a real test device, the test results may note be 100% reproducible,
+and sporadic test failures may occur under certain circumstances.
+It may be necessary to introduce a certain delay between test
+operations. To do so, set the environment variable `DIO_TEST_DELAY` to a
+positive integer that determines the delay (in microseconds) after each
+`io_submit()` operation. The default delay is 10 microseconds.
+
+*Note:* `DIO_TEST_DEV` doesn't have to be set during compilation of
+`directio-test`. This used to be the case in previous versions of
+multipath-tools. Previously, it was possible to set `DIO_TEST_DEV` in a file
+`tests/directio_test_dev`. This is not supported any more.
+
 ## Adding tests
 
 The unit tests are based on the [cmocka test framework](https://cmocka.org/),
diff --git a/tests/directio.c b/tests/directio.c
index d5f84f1..763929e 100644
--- a/tests/directio.c
+++ b/tests/directio.c
@@ -34,6 +34,8 @@ struct io_event mock_events[AIO_GROUP_SIZE]; /* same as the 
checker max */
 int ev_off = 0;
 struct timespec zero_timeout = { .tv_sec = 0 };
 struct timespec full_timeout = { .tv_sec = -1 };
+const char *test_dev = NULL;
+unsigned int test_delay = 10000;
 
 #ifdef __GLIBC__
 #define ioctl_request_t unsigned long
@@ -45,12 +47,13 @@ int REAL_IOCTL(int fd, ioctl_request_t request, void *argp);
 
 int WRAP_IOCTL(int fd, ioctl_request_t request, void *argp)
 {
-#ifdef DIO_TEST_DEV
-       mock_type(int);
-       return REAL_IOCTL(fd, request, argp);
-#else
        int *blocksize = (int *)argp;
 
+       if (test_dev) {
+               mock_type(int);
+               return REAL_IOCTL(fd, request, argp);
+       }
+
        assert_int_equal(fd, test_fd);
        /*
         * On MUSL libc, the "request" arg is an int (glibc: unsigned long).
@@ -64,88 +67,80 @@ int WRAP_IOCTL(int fd, ioctl_request_t request, void *argp)
        assert_non_null(blocksize);
        *blocksize = mock_type(int);
        return 0;
-#endif
 }
 
 int REAL_FCNTL (int fd, int cmd, long arg);
 
 int WRAP_FCNTL (int fd, int cmd, long arg)
 {
-#ifdef DIO_TEST_DEV
-       return REAL_FCNTL(fd, cmd, arg);
-#else
+       if (test_dev)
+               return REAL_FCNTL(fd, cmd, arg);
        assert_int_equal(fd, test_fd);
        assert_int_equal(cmd, F_GETFL);
        return O_DIRECT;
-#endif
 }
 
 int __real___fxstat(int ver, int fd, struct stat *statbuf);
 
 int __wrap___fxstat(int ver, int fd, struct stat *statbuf)
 {
-#ifdef DIO_TEST_DEV
-       return __real___fxstat(ver, fd, statbuf);
-#else
+       if (test_dev)
+               return __real___fxstat(ver, fd, statbuf);
+
        assert_int_equal(fd, test_fd);
        assert_non_null(statbuf);
        memset(statbuf, 0, sizeof(struct stat));
        return 0;
-#endif
+
 }
 
 int __real_io_setup(int maxevents, io_context_t *ctxp);
 
 int __wrap_io_setup(int maxevents, io_context_t *ctxp)
 {
-       ioctx_count++;
-#ifdef DIO_TEST_DEV
        int ret = mock_type(int);
-       assert_int_equal(ret, __real_io_setup(maxevents, ctxp));
+
+       if (test_dev)
+               assert_int_equal(ret, __real_io_setup(maxevents, ctxp));
+       ioctx_count++;
        return ret;
-#else
-       return mock_type(int);
-#endif
 }
 
 int __real_io_destroy(io_context_t ctx);
 
 int __wrap_io_destroy(io_context_t ctx)
 {
-       ioctx_count--;
-#ifdef DIO_TEST_DEV
        int ret = mock_type(int);
-       assert_int_equal(ret, __real_io_destroy(ctx));
+
+       ioctx_count--;
+       if (test_dev)
+               assert_int_equal(ret, __real_io_destroy(ctx));
+
        return ret;
-#else
-       return mock_type(int);
-#endif
 }
 
 int __real_io_submit(io_context_t ctx, long nr, struct iocb *ios[]);
 
 int __wrap_io_submit(io_context_t ctx, long nr, struct iocb *ios[])
 {
-#ifdef DIO_TEST_DEV
-       struct timespec dev_delay = { .tv_nsec = 100000 };
        int ret = mock_type(int);
-       assert_int_equal(ret, __real_io_submit(ctx, nr, ios));
-       nanosleep(&dev_delay, NULL);
+
+       if (test_dev) {
+               struct timespec dev_delay = { .tv_nsec = test_delay };
+               assert_int_equal(ret, __real_io_submit(ctx, nr, ios));
+               nanosleep(&dev_delay, NULL);
+       }
        return ret;
-#else
-       return mock_type(int);
-#endif
 }
 
 int __real_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event 
*evt);
 
 int __wrap_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt)
 {
-#ifdef DIO_TEST_DEV
-       return __real_io_cancel(ctx, iocb, evt);
-#else
-       return 0;
-#endif
+       if (test_dev)
+               return __real_io_cancel(ctx, iocb, evt);
+       else
+               return 0;
 }
 
 int REAL_IO_GETEVENTS(io_context_t ctx, long min_nr, long nr,
@@ -155,38 +150,43 @@ int WRAP_IO_GETEVENTS(io_context_t ctx, long min_nr, long 
nr,
                        struct io_event *events, struct timespec *timeout)
 {
        int nr_evs;
-#ifndef DIO_TEST_DEV
        struct timespec *sleep_tmo;
        int i;
        struct io_event *evs;
-#endif
 
        assert_non_null(timeout);
        nr_evs = mock_type(int);
        assert_true(nr_evs <= nr);
        if (!nr_evs)
                return 0;
-#ifdef DIO_TEST_DEV
-       mock_ptr_type(struct timespec *);
-       mock_ptr_type(struct io_event *);
-       assert_int_equal(nr_evs, REAL_IO_GETEVENTS(ctx, min_nr, nr_evs,
-                                                  events, timeout));
-#else
-       sleep_tmo = mock_ptr_type(struct timespec *);
-       if (sleep_tmo) {
-               if (sleep_tmo->tv_sec < 0)
-                       nanosleep(timeout, NULL);
-               else
-                       nanosleep(sleep_tmo, NULL);
+       if (test_dev) {
+               int n = 0;
+               mock_ptr_type(struct timespec *);
+               mock_ptr_type(struct io_event *);
+
+               condlog(2, "min_nr = %ld nr_evs = %d", min_nr, nr_evs);
+               while (n < nr_evs) {
+                       min_nr = min_nr <= nr_evs - n ? min_nr : nr_evs - n;
+                       n += REAL_IO_GETEVENTS(ctx, min_nr, nr_evs - n,
+                                              events + n, timeout);
+               }
+               assert_int_equal(nr_evs, n);
+       } else {
+               sleep_tmo = mock_ptr_type(struct timespec *);
+               if (sleep_tmo) {
+                       if (sleep_tmo->tv_sec < 0)
+                               nanosleep(timeout, NULL);
+                       else
+                               nanosleep(sleep_tmo, NULL);
+               }
+               if (nr_evs < 0) {
+                       errno = -nr_evs;
+                       return -1;
+               }
+               evs = mock_ptr_type(struct io_event *);
+               for (i = 0; i < nr_evs; i++)
+                       events[i] = evs[i];
        }
-       if (nr_evs < 0) {
-               errno = -nr_evs;
-               return -1;
-       }
-       evs = mock_ptr_type(struct io_event *);
-       for (i = 0; i < nr_evs; i++)
-               events[i] = evs[i];
-#endif
        ev_off -= nr_evs;
        return nr_evs;
 }
@@ -259,10 +259,9 @@ static void do_libcheck_init(struct checker *c, int 
blocksize,
        assert_non_null(ct->req);
        if (req)
                *req = ct->req;
-#ifndef DIO_TEST_DEV
-       /* don't check fake blocksize on real devices */
-       assert_int_equal(ct->req->blksize, blocksize);
-#endif
+       if (!test_dev)
+               /* don't check fake blocksize on real devices */
+               assert_int_equal(ct->req->blksize, blocksize);
 }
 
 static int is_checker_running(struct checker *c)
@@ -583,11 +582,11 @@ static void test_async_timeout_cancel_failed(void **state)
        do_check_state(&c[1], 0, 2, PATH_PENDING);
        return_io_getevents_none();
        do_check_state(&c[0], 0, 2, PATH_DOWN);
-#ifndef DIO_TEST_DEV
-       /* can't pick which even gets returned on real devices */
-       return_io_getevents_nr(NULL, 1, &reqs[1], &res[1]);
-       do_check_state(&c[1], 0, 2, PATH_UP);
-#endif
+       if (!test_dev) {
+               /* can't pick which even gets returned on real devices */
+               return_io_getevents_nr(NULL, 1, &reqs[1], &res[1]);
+               do_check_state(&c[1], 0, 2, PATH_UP);
+       }
        return_io_getevents_none();
        do_check_state(&c[0], 0, 2, PATH_DOWN);
        assert_true(is_checker_running(&c[0]));
@@ -663,12 +662,11 @@ static void test_check_state_blksize(void **state)
        int blksize[] = {4096, 1024, 512};
        struct async_req *reqs[3];
        int res[] = {0,1,0};
-#ifdef DIO_TEST_DEV
-       /* can't pick event return state on real devices */
        int chk_state[] = {PATH_UP, PATH_UP, PATH_UP};
-#else
-       int chk_state[] = {PATH_UP, PATH_DOWN, PATH_UP};
-#endif
+
+       /* can't pick event return state on real devices */
+       if (!test_dev)
+               chk_state[1] = PATH_DOWN;
 
        assert_true(list_empty(&aio_grp_list));
        will_return(__wrap_io_setup, 0);
@@ -718,20 +716,38 @@ static void test_check_state_async(void **state)
 
 static int setup(void **state)
 {
-#ifdef DIO_TEST_DEV
-       test_fd = open(DIO_TEST_DEV, O_RDONLY);
-       if (test_fd < 0)
-               fail_msg("cannot open %s: %m", DIO_TEST_DEV);
-#endif
+       char *dl = getenv("DIO_TEST_DELAY");
+       test_dev = getenv("DIO_TEST_DEV");
+
+       if (test_dev) {
+               condlog(2, "%s: opening %s", __func__, test_dev);
+               test_fd = open(test_dev, O_RDONLY);
+               if (dl) {
+                       char *e;
+                       long int d = strtol(dl, &e, 10);
+
+                       if (*e == '\0' && d >= 0 && (d * 1000) < (long)UINT_MAX)
+                               test_delay = d * 1000;
+                       else {
+                               condlog(1, "DIO_TEST_DELAY=%s is invalid", dl);
+                               return 1;
+                       }
+               }
+               condlog(2, "%s: delay = %u us", __func__, test_delay / 1000);
+       }
+       if (test_fd < 0) {
+               fail_msg("cannot open %s: %m", test_dev);
+               return 1;
+       }
        return 0;
 }
 
 static int teardown(void **state)
 {
-#ifdef DIO_TEST_DEV
-       assert_true(test_fd > 0);
-       assert_int_equal(close(test_fd), 0);
-#endif
+       if (test_dev) {
+               assert_true(test_fd > 0);
+               assert_int_equal(close(test_fd), 0);
+       }
        return 0;
 }
 
@@ -762,7 +778,7 @@ int main(void)
 {
        int ret = 0;
 
-       init_test_verbosity(5);
+       init_test_verbosity(2);
        ret += test_directio();
        return ret;
 }
-- 
2.45.2


Reply via email to