From 84e44dd0122cc4e532206bad01a25629deac3d9f Mon Sep 17 00:00:00 2001 From: iamamyth 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 | 37 ++++++++++---- 3 files changed, 128 insertions(+), 9 deletions(-) create mode 100755 tests/create_test_disks.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 26b2505df5..6f7840dd1f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -163,6 +163,8 @@ jobs: run: ${{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 0000000000..8e17c732d9 --- /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 " + return 1 + fi + echo "$datadir/mnt-$(basename "$dev")" +} + +create_device() { + local -r datadir="$1" + if [[ -z "$datadir" ]]; then + echo_err "Usage: create_device " + 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 " + 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 040af4f47c..55f759ed9f 100644 --- a/tests/unit_tests/is_hdd.cpp +++ b/tests/unit_tests/is_hdd.cpp @@ -1,17 +1,36 @@ #include "common/util.h" +#include #include #include +#include /* required to output boost::optional in assertions */ + +#ifndef GTEST_SKIP +#include +#define SKIP_TEST(reason) do {std::cerr << "Skipping test: " << reason << std::endl; return;} while(0) +#else +#define SKIP_TEST(reason) GTEST_SKIP() << reason +#endif #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) + SKIP_TEST("No rotational disk device configured"); + EXPECT_EQ(tools::is_hdd(hdd), boost::optional(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) + SKIP_TEST("No SSD device configured"); + EXPECT_EQ(tools::is_hdd(ssd), boost::optional(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("")); +}