From f48a73d91937c9ed654e3dd740cff86f3966dd23 Mon Sep 17 00:00:00 2001
From: iamamyth <user@example.com>
Date: Fri, 17 Jan 2025 02:48:48 -0800
Subject: [PATCH] tests: Fix tools::is_hdd unit tests

Correct the unit tests for tools::is_hdd to avoid making assumptions
about the configuration of a particular device based solely on the
value of the __GLIBC__ preprocessor flag. Instead, rely on the
test invoker to provide paths for devices of specific types via
the process environment, thereby avoiding faulty assumptions and
improving the specificity of test assertions. To ensure appropriate
devices exist, add a script, tests/create_test_disks.sh, which
configures loopback devices mirroring relevant configurations.
---
 .github/workflows/build.yml |  2 +
 tests/create_test_disks.sh  | 98 +++++++++++++++++++++++++++++++++++++
 tests/unit_tests/is_hdd.cpp | 30 ++++++++----
 3 files changed, 121 insertions(+), 9 deletions(-)
 create mode 100755 tests/create_test_disks.sh

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index b9729e07a..242c1e228 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -213,6 +213,8 @@ jobs:
       run: sudo ${{env.APT_INSTALL_LINUX}}
     - name: install Python dependencies
       run: pip install requests psutil monotonic zmq deepdiff
+    - name: create dummy disk drives for testing
+      run: tests/create_test_disks.sh >> $GITHUB_ENV
     - name: tests
       env:
         CTEST_OUTPUT_ON_FAILURE: ON
diff --git a/tests/create_test_disks.sh b/tests/create_test_disks.sh
new file mode 100755
index 000000000..8e17c732d
--- /dev/null
+++ b/tests/create_test_disks.sh
@@ -0,0 +1,98 @@
+#!/bin/bash
+
+set -e
+
+LOOP_DEVICE_MAJOR=7
+LOOP_DEVICE_MIN_ID=100
+
+echo_err() {
+    echo "$@" >&2
+}
+
+root_exec() {
+    if [[ $EUID -ne 0 ]]; then
+        ${ROOT_EXEC_CMD:-sudo} "$@"
+    else
+        "$@"
+    fi
+}
+
+create_device_node() {
+    local -r last_id=$(find /dev/ -name 'loop[0-9]*' -printf '%f\n' | sed 's/^loop//' | sort -r -n | head -1)
+    local id=$((last_id + 1))
+    if [[ "$id" -lt "$LOOP_DEVICE_MIN_ID" ]]; then
+        id="$LOOP_DEVICE_MIN_ID"
+    fi
+    local path
+    for (( i=0; i<10; i++ )); do
+        path="/dev/loop$id"
+        if [[ ! -e "$path" ]] && root_exec mknod "$path" b "$LOOP_DEVICE_MAJOR" "$id"; then
+            echo "$path"
+            return 0
+        fi
+        $((id++))
+    done
+    return 1
+}
+
+device_mountpoint() {
+    local -r datadir="$1"
+    local -r dev="$2"
+    if [[ -z "$datadir" || -z "$dev" ]]; then
+        echo_err "Usage: device_mountpoint <data dir> <device>"
+        return 1
+    fi
+    echo "$datadir/mnt-$(basename "$dev")"
+}
+
+create_device() {
+    local -r datadir="$1"
+    if [[ -z "$datadir" ]]; then
+        echo_err "Usage: create_device <data dir>"
+        return 1
+    fi
+    local -r dev=$(create_device_node)
+    local -r fs="$datadir/$(basename "$dev").vhd"
+    local -r mountpoint=$(device_mountpoint "$datadir" "$dev")
+    echo_err
+    echo_err "# Device $dev"
+    dd if=/dev/zero of="$fs" bs=64K count=128 >/dev/null 2>&1
+    root_exec losetup "$dev" "$fs"
+    root_exec mkfs.ext4 "$dev" >/dev/null 2>&1
+    mkdir "$mountpoint"
+    root_exec mount "$dev" "$mountpoint"
+    echo "$dev"
+}
+
+# Unused by default, but helpful for local development
+destroy_device() {
+    local -r datadir="$1"
+    local -r dev="$2"
+    if [[ -z "$datadir" || -z "$dev" ]]; then
+        echo_err "Usage: destroy_device <data dir> <device>"
+        return 1
+    fi
+    echo_err "Destroying device $dev"
+    root_exec umount $(device_mountpoint "$datadir" "$dev")
+    root_exec losetup -d "$dev"
+    root_exec rm "$dev"
+}
+
+block_device_path() {
+    device_name=$(basename "$1")
+    device_minor=${device_name/#loop}
+    echo "/sys/dev/block/$LOOP_DEVICE_MAJOR:$device_minor"
+}
+
+tmpdir=$(mktemp --tmpdir -d monerotest.XXXXXXXX)
+echo_err "Creating devices using temporary directory: $tmpdir"
+
+dev_rot=$(create_device "$tmpdir")
+bdev_rot=$(block_device_path "$dev_rot")
+echo 1 | root_exec tee "$bdev_rot/queue/rotational" >/dev/null
+echo MONERO_TEST_DEVICE_HDD=$(device_mountpoint "$tmpdir" "$dev_rot")
+
+dev_ssd=$(create_device "$tmpdir")
+bdev_ssd=$(block_device_path "$dev_ssd")
+echo 0 | root_exec tee "$bdev_ssd/queue/rotational" >/dev/null
+echo MONERO_TEST_DEVICE_SSD=$(device_mountpoint "$tmpdir" "$dev_ssd")
diff --git a/tests/unit_tests/is_hdd.cpp b/tests/unit_tests/is_hdd.cpp
index 040af4f47..7d4344d0a 100644
--- a/tests/unit_tests/is_hdd.cpp
+++ b/tests/unit_tests/is_hdd.cpp
@@ -1,17 +1,29 @@
 #include "common/util.h"
+#include <cstdlib>
 #include <string>
 #include <gtest/gtest.h>
+#include <boost/optional/optional_io.hpp> /* required to output boost::optional in assertions */
 
 #if defined(__GLIBC__)
-TEST(is_hdd, linux_os_root)
-{
-  std::string path = "/";
-  EXPECT_TRUE(tools::is_hdd(path.c_str()) != boost::none);
+TEST(is_hdd, rotational_drive) {
+  const char *hdd = std::getenv("MONERO_TEST_DEVICE_HDD");
+  if (hdd == nullptr)
+    GTEST_SKIP() << "No rotational disk device configured";
+  EXPECT_EQ(tools::is_hdd(hdd), boost::optional<bool>(true));
 }
-#else
-TEST(is_hdd, unknown_os)
-{
-  std::string path = "";
-  EXPECT_FALSE(tools::is_hdd(path.c_str()) != boost::none);
+
+TEST(is_hdd, ssd) {
+  const char *ssd = std::getenv("MONERO_TEST_DEVICE_SSD");
+  if (ssd == nullptr)
+    GTEST_SKIP() << "No SSD device configured";
+  EXPECT_EQ(tools::is_hdd(ssd), boost::optional<bool>(false));
+}
+
+TEST(is_hdd, unknown_attrs) {
+  EXPECT_EQ(tools::is_hdd("/dev/null"), boost::none);
 }
 #endif
+TEST(is_hdd, stability)
+{
+  EXPECT_NO_THROW(tools::is_hdd(""));
+}