Merge pull request #11 from ubicloud/pykello/coverage

Add test coverage data & increase coverage.
ubicloud · Jan 27, 2024 · 1269730 · 1269730
2 parents 4620213 + 2495174
commit 1269730
Show file tree

Hide file tree

Showing 7 changed files with 261 additions and 64 deletions.
diff --git a/.github/scripts/check-coverage.sh b/.github/scripts/check-coverage.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+make coverage
+
+target=70.0
+p=$(lcov --summary coverage.info | grep "lines" | grep -Eo "[0-9]+\.[0-9]+")
+if (( $(echo "$p < $target" | bc -l) )); then
+    echo "Error: Coverage $p% is less than $target%"
+    exit 1 
+fi
diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
@@ -34,7 +34,7 @@ jobs:
       - name: Install dependencies
         run: |
           sudo apt-get update
-          sudo apt install liburing-dev
+          sudo apt install liburing-dev lcov
           sudo spdk/scripts/pkgdep.sh
       - name: Configure
         run: spdk/configure --with-crypto --with-vhost --target-arch=corei7 --prefix=$INSTALL_PREFIX --pydir=$INSTALL_PREFIX/python/lib --disable-unit-tests --disable-tests --disable-examples
@@ -47,14 +47,20 @@ jobs:
           cp -R python/* $INSTALL_PREFIX/python/
           mkdir $INSTALL_PREFIX/scripts
           cp scripts/rpc.py $INSTALL_PREFIX/scripts
+      - name: Run tests and check coverage
+        run: |
+          sudo spdk/scripts/setup.sh
+          export SPDK_PATH=$INSTALL_PREFIX
+          make clean
+          make COVERAGE=true
+          make check
+          .github/scripts/check-coverage.sh
       - name: Build bdev_ubi
         run: |
-          SPDK_PATH=$INSTALL_PREFIX make
+          export SPDK_PATH=$INSTALL_PREFIX
+          make clean
+          make
           cp build/bin/vhost_ubi $INSTALL_PREFIX/bin/
-      - name: Run tests
-        run: |
-          sudo spdk/scripts/setup.sh
-          SPDK_PATH=$INSTALL_PREFIX make check
       - name: Package
         run: |
           cd $INSTALL_PREFIX/..

diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,5 @@
 build/*
 .vscode/*
 valgrind.log
+coverage.info
+coverage_report/*
diff --git a/Makefile b/Makefile
@@ -4,6 +4,7 @@ APP_DIR := app
 TEST_DIR := test
 OBJ_DIR := build/obj
 BIN_DIR := build/bin
+TEST_BDEVS := --bdev ubi0 --bdev ubi_nosync --bdev ubi_directio --bdev ubi_copy_on_read
 
 ifneq ($(MAKECMDGOALS),format)
 PKG_CONFIG_PATH = $(SPDK_PATH)/lib/pkgconfig
@@ -20,6 +21,10 @@ CFLAGS := -D_GNU_SOURCE -Iinclude -Wall -g -O3 -I$(SPDK_PATH)/include
 LDFLAGS := -Wl,--whole-archive,-Bstatic $(SPDK_DPDK_LIB) -Wl,--no-whole-archive -luring -Wl,-Bdynamic $(SYS_LIB)
 endif
 
+ifeq ($(COVERAGE),true)
+    CFLAGS += -fprofile-arcs -ftest-coverage
+endif
+
 # Automatically generate a list of source files (.c) and object files (.o)
 SRCS := $(wildcard $(SRC_DIR)/*.c)
 OBJS := $(SRCS:$(SRC_DIR)/%.c=$(OBJ_DIR)/%.o)
@@ -58,14 +63,18 @@ $(BIN_DIR)/test_disk.raw: $(BIN_DIR)/test_image.raw
 	truncate --size 100M $@
 
 check:
-	sudo ./build/bin/test_ubi --cpumask [0,1,2] --json test/test_conf.json
+	sudo ./build/bin/test_ubi --cpumask [0,1,2] --json test/test_conf.json $(TEST_BDEVS)
 
 valgrind:
-	sudo valgrind ./build/bin/memcheck_ubi --cpumask [0] --json test/test_conf.json
+	sudo valgrind ./build/bin/memcheck_ubi --cpumask [0] --json test/test_conf.json $(TEST_BDEVS)
+
+coverage:
+	lcov --capture --directory . --exclude=`pwd`/test/'*.c' --no-external --output-file coverage.info > /dev/null
+	genhtml coverage.info --output-directory coverage_report
 
 # Clean up build artifacts
 clean:
-	@rm -rf $(OBJ_DIR) $(BIN_DIR)
+	@rm -rf $(OBJ_DIR) $(BIN_DIR) coverage.info coverage_report
 
 # Automatically format source files
 format:

diff --git a/test/memcheck_ubi.c b/test/memcheck_ubi.c
@@ -14,11 +14,14 @@
 
 #define BLOCK_SIZE 512
 #define MAX_OPS 5000
+#define MAX_BDEVS 10
 
 struct {
-    char *bdev_name;
+    char *bdev_names[MAX_BDEVS];
+    int n_bdevs;
 } g_opts;
 
+size_t g_bdevs_tested = 0;
 struct test_state {
     struct spdk_bdev_desc *bdev_desc;
     struct spdk_io_channel *ch;
@@ -43,6 +46,7 @@ static struct option g_cmdline_opts[] = {{
                                          {.name = NULL}};
 
 static void enqueue_io_ops(void *arg);
+static void open_bdev(void *arg);
 
 #define continue_with_fn(fn)                                                             \
     {                                                                                    \
@@ -70,8 +74,6 @@ static void fail_tests(void *arg) {
 }
 
 static void succeed_tests(void *arg) {
-    close_bdev();
-
     SPDK_NOTICELOG("Tests succeeded.\n");
     spdk_app_stop(0);
 }
@@ -86,9 +88,11 @@ static void io_completion_cb(struct spdk_bdev_io *bdev_io, bool success, void *a
     if (g_test_state.n_ops_finished != g_test_state.n_ops_queued)
         return;
 
-    // reached target op count
-    if (g_test_state.n_ops_finished >= g_test_state.n_ops_target)
-        continue_with_fn(succeed_tests);
+    // reached target op count. continue with testing the next bdev.
+    if (g_test_state.n_ops_finished >= g_test_state.n_ops_target) {
+        close_bdev();
+        continue_with_fn(open_bdev);
+    }
 
     continue_with_fn(enqueue_io_ops);
 }
@@ -144,7 +148,14 @@ static void enqueue_io_ops(void *arg) {
 }
 
 static void open_bdev(void *arg) {
-    char *name = g_opts.bdev_name ? g_opts.bdev_name : DEFAULT_BDEV_NAME;
+    if (g_bdevs_tested >= g_opts.n_bdevs) {
+        continue_with_fn(succeed_tests);
+    }
+
+    char *name = g_opts.bdev_names[g_bdevs_tested++];
+    memset(&g_test_state, 0, sizeof(g_test_state));
+    g_test_state.n_ops_target = MAX_OPS;
+
     int rc = spdk_bdev_open_ext(name, true, ubi_event_cb, NULL, &g_test_state.bdev_desc);
     if (rc < 0) {
         SPDK_ERRLOG("Could not open bdev %s: %s\n", name, strerror(-rc));
@@ -160,18 +171,18 @@ static void open_bdev(void *arg) {
     continue_with_fn(enqueue_io_ops);
 };
 
-static void start_tests(void *arg) {
-    memset(&g_test_state, 0, sizeof(g_test_state));
-    g_test_state.n_ops_target = MAX_OPS;
-    continue_with_fn(open_bdev);
-}
+static void start_tests(void *arg) { continue_with_fn(open_bdev); }
 
 static void usage(void) { printf("  -bdev Block device to be used for testing.\n"); }
 
 static int parse_arg(int argc, char *argv) {
     switch (argc) {
     case MEMCHECK_OPTION_BDEV:
-        g_opts.bdev_name = strdup(argv);
+        if (g_opts.n_bdevs >= MAX_BDEVS) {
+            fprintf(stderr, "Too many bdevs.\n");
+            exit(-1);
+        }
+        g_opts.bdev_names[g_opts.n_bdevs++] = strdup(argv);
         break;
     default:
         return -EINVAL;
@@ -191,12 +202,19 @@ int main(int argc, char **argv) {
         exit(rc);
     }
 
+    if (g_opts.n_bdevs == 0) {
+        g_opts.n_bdevs = 1;
+        g_opts.bdev_names[0] = strdup(DEFAULT_BDEV_NAME);
+    }
+
     rc = spdk_app_start(&opts, start_tests, NULL);
     if (rc) {
         SPDK_ERRLOG("Error occured while testing bdev_ubi.\n");
     }
 
-    free(g_opts.bdev_name);
+    for (int i = 0; i < g_opts.n_bdevs; i++)
+        free(g_opts.bdev_names[i]);
+
     spdk_app_fini();
 
     return rc;

diff --git a/test/test_conf.json b/test/test_conf.json
@@ -19,7 +19,68 @@
             "image_path": "build/bin/test_image.raw",
             "stripe_size_kb": 1024,
             "copy_on_read": false,
-            "directio": true
+            "directio": false,
+            "no_sync": false
+          }
+        },
+        {
+          "method": "bdev_malloc_create",
+          "params": {
+            "name": "malloc1",
+            "block_size": 512,
+            "num_blocks": 204800
+          }
+        },
+        {
+          "method": "bdev_ubi_create",
+          "params": {
+            "name": "ubi_copy_on_read",
+            "base_bdev": "malloc1",
+            "image_path": "build/bin/test_image.raw",
+            "stripe_size_kb": 1024,
+            "copy_on_read": true,
+            "directio": false,
+            "no_sync": false
+          }
+        },
+        {
+          "method": "bdev_malloc_create",
+          "params": {
+            "name": "malloc2",
+            "block_size": 512,
+            "num_blocks": 204800
+          }
+        },
+        {
+          "method": "bdev_ubi_create",
+          "params": {
+            "name": "ubi_directio",
+            "base_bdev": "malloc2",
+            "image_path": "build/bin/test_image.raw",
+            "stripe_size_kb": 1024,
+            "copy_on_read": false,
+            "directio": true,
+            "no_sync": false
+          }
+        },
+        {
+          "method": "bdev_malloc_create",
+          "params": {
+            "name": "malloc3",
+            "block_size": 512,
+            "num_blocks": 204800
+          }
+        },
+        {
+          "method": "bdev_ubi_create",
+          "params": {
+            "name": "ubi_nosync",
+            "base_bdev": "malloc3",
+            "image_path": "build/bin/test_image.raw",
+            "stripe_size_kb": 1024,
+            "copy_on_read": false,
+            "directio": false,
+            "no_sync": true
           }
         },
         {