Add unit tests for SelfTest adaptive ETA algorithm

Co-authored-by: ashaduri <2302268+ashaduri@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ashaduri/gsmartcontrol/sessions/8b4b77f5-beb6-4c32-bf49-93d641714685

src/applib/selftest.cpp

@@ -89,15 +89,46 @@ std::chrono::seconds SelfTest::get_remaining_seconds() const
 {
 	using namespace std::literals;
 
+	// Use adaptive estimation if we have observed at least one completed segment.
+	// This works for all drive types including NVMe (which may not report total duration).
+	if (!segment_durations_.empty()) {
+		// Calculate average duration of observed segments
+		double sum = 0.0;
+		for (const auto& duration : segment_durations_) {
+			sum += duration;
+		}
+		const double avg_segment_duration = sum / segment_durations_.size();
+
+		// Estimate remaining time based on observed average and remaining segments
+		// remaining_percent_ goes from 100 (start) to 0 (end), in 10% decrements
+		const int remaining_segments = (remaining_percent_ + 9) / 10;  // round up
+		const double estimated_remaining = avg_segment_duration * remaining_segments - timer_.elapsed();
+
+		const auto rem_rounded = static_cast<int64_t>(std::round(estimated_remaining));
+		if (rem_rounded < 0) {
+			return -1s;  // estimate exhausted; return unknown
+		}
+		return std::chrono::seconds(rem_rounded);
+	}
+
+	// Fall back to drive's initial estimate when we don't have observed data yet
 	const std::chrono::seconds total = get_min_duration_seconds();
 	if (total <= 0s)
 		return -1s;  // unknown
 
-	const double gran = (double(total.count()) / 9.);  // seconds per 10%
+	// seconds per 10% (drive estimate)
+	const double gran = (double(total.count()) / 9.);
 	// since remaining_percent_ may be manually set to 100, we limit from the above.
 	const double rem_seconds_at_last_change = std::min(double(total.count()), gran * remaining_percent_ / 10.);
 	const double rem = rem_seconds_at_last_change - timer_.elapsed();
-	return std::chrono::seconds(std::max(int64_t(0), (int64_t)std::round(rem)));  // don't return negative values.
+	const auto rem_rounded = static_cast<int64_t>(std::round(rem));
+	// If the estimated time for the current percentage has elapsed but the drive hasn't
+	// progressed, the drive's estimate was inaccurate. Return -1 (unknown) instead of 0
+	// to avoid misleading "ETA: 0 sec" which could persist for hours.
+	if (rem_rounded < 0) {
+		return -1s;
+	}
+	return std::chrono::seconds(rem_rounded);
 }
 
 
@@ -492,6 +523,15 @@ hz::ExpectedVoid<SelfTestExecutionError> SelfTest::update(const std::shared_ptr<
 	// and reaches 00% on completion. That's 9 pieces.
 	if (status_ == SelfTestStatus::InProgress) {
 		if (remaining_percent_ != last_seen_percent_) {
+			// Record the duration of the completed segment for adaptive ETA calculation.
+			// Skip the first segment (typically 90→80) as it may be instant or partially
+			// completed when monitoring begins, which would skew the average.
+			if (first_segment_seen_) {
+				const double elapsed = timer_.elapsed();
+				segment_durations_.push_back(elapsed);
+			} else {
+				first_segment_seen_ = true;  // Mark that we've seen the first transition
+			}
 			last_seen_percent_ = remaining_percent_;
 			timer_.start();  // restart the timer
 		}

src/applib/selftest.h

@@ -18,6 +18,7 @@ Copyright:
 #include <cstdint>
 #include <chrono>
 #include <unordered_map>
+#include <vector>
 
 #include "storage_device.h"
 #include "command_executor.h"
@@ -126,7 +127,15 @@ class SelfTest {
 
 
 		/// Get estimated time of completion for the test.
-		/// \return -1 if N/A or unknown. Note that 0 is a valid value.
+		/// The estimation uses an adaptive algorithm:
+		/// - Initially uses the drive's reported test duration estimate
+		/// - After completing one or more 10% segments, switches to using the observed
+		///   average segment duration to predict remaining time
+		/// - This provides more accurate ETAs when the drive's estimate is inaccurate
+		///   (e.g., under load or with drives that consistently under/overestimate)
+		/// \return -1 if N/A or unknown (including when the drive's estimated duration has been
+		/// exceeded without a percentage change, which means the estimate was inaccurate).
+		/// Note that 0 is a valid value meaning the test is finishing right now.
 		[[nodiscard]] std::chrono::seconds get_remaining_seconds() const;
 
 
@@ -180,6 +189,8 @@ class SelfTest {
 		std::chrono::seconds poll_in_seconds_ = std::chrono::seconds(-1);  ///< The user is asked to poll after this much seconds have passed.
 
 		Glib::Timer timer_;  ///< Counts time since the last percent change
+		std::vector<double> segment_durations_;  ///< Actual durations of completed 10% segments (in seconds), for adaptive ETA calculation
+		bool first_segment_seen_ = false;  ///< Whether we've observed the first percentage change (to skip the potentially instant/partial first segment)
 
 };
 

src/applib/storage_property_descr.cpp

@@ -115,7 +115,6 @@ bool storage_property_autoset_description(StorageProperty& p, StorageDeviceDetec
 				found = auto_set(p, "ata_smart_attributes/revision", p.displayable_name.c_str());
 				if (!found) {
 					auto_set_ata_attribute_description(p, device_type);
-					storage_property_ata_attribute_humanize_ssd_writes(p);
 					found = true;  // true, because auto_set_attr() may set "Unknown attribute", which is still "found".
 				}
 				break;

src/applib/storage_property_descr_ata_attribute.cpp

@@ -22,7 +22,6 @@ Copyright:
 //#include "warning_colors.h"
 #include "storage_property_descr_helpers.h"
 #include "hz/string_num.h"
-#include "hz/format_unit.h"  // format_size
 
 
 namespace {
@@ -1365,104 +1364,5 @@ void storage_property_ata_attribute_autoset_warning(StorageProperty& p)
 
 
 
-void storage_property_ata_attribute_humanize_ssd_writes(StorageProperty& p)
-{
-	if (p.section != StoragePropertySection::AtaAttributes || !p.is_value_type<AtaStorageAttribute>()) {
-		return;
-	}
-
-	const auto& attr = p.get_value<AtaStorageAttribute>();
-
-	// Skip if readable_value is already set (e.g., by parser or for GiB attributes)
-	if (!p.readable_value.empty()) {
-		return;
-	}
-
-	// Standard sector size (512 bytes)
-	constexpr uint64_t bytes_per_sector = 512;
-	constexpr uint64_t mib_32 = 32ULL * 1024ULL * 1024ULL;
-	constexpr uint64_t gib = 1024ULL * 1024ULL * 1024ULL;
-
-	// Match attribute by ID and reported name to handle vendor-specific attributes
-	const int32_t id = attr.id;
-	const std::string& name = p.reported_name;
-	std::optional<uint64_t> bytes;
-
-	// Write attributes - these need humanization most
-	// Attribute 199: Write_Sectors_Tot_Ct (Indilinx Barefoot SSDs)
-	// Total count of written sectors
-	if (id == 199 && name == "Write_Sectors_Tot_Ct") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
-	}
-	// Attribute 225: Host_Writes_32MiB (Intel SSDs)
-	else if (id == 225 && name == "Host_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 241: Host_Writes_32MiB (various SSDs)
-	// Raw value increased by 1 for every 32 MiB written
-	else if (id == 241 && name == "Host_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 243: Host_Writes_32MiB (SanDisk SSDs)
-	else if (id == 243 && name == "Host_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 245: Flash_Writes_32MiB (Innodisk SSDs)
-	else if (id == 245 && name == "Flash_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 245: TLC_Writes_32MiB (SiliconMotion SSDs)
-	else if (id == 245 && name == "TLC_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 246: SLC_Writes_32MiB (SiliconMotion SSDs)
-	else if (id == 246 && name == "SLC_Writes_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 246: Total_Host_Sector_Write (Crucial/Micron SSDs)
-	// Total number of sectors written by the host system
-	else if (id == 246 && name == "Total_Host_Sector_Write") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
-	}
-	// Attribute 249: NAND_Writes_1GiB (Intel SSDs)
-	// Note: The raw value is the count, not already in GiB
-	else if (id == 249 && name == "NAND_Writes_1GiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
-	}
-	// Attribute 249: Total_NAND_Prog_Ct_GiB (OCZ SSDs)
-	else if (id == 249 && name == "Total_NAND_Prog_Ct_GiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
-	}
-
-	// Read attributes - also humanize for consistency
-	// Attribute 198: Read_Sectors_Tot_Ct (Indilinx Barefoot SSDs)
-	else if (id == 198 && name == "Read_Sectors_Tot_Ct") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * bytes_per_sector;
-	}
-	// Attribute 226: Host_Reads_32MiB (Intel SSDs)
-	else if (id == 226 && name == "Host_Reads_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 242: Host_Reads_32MiB (Intel SSDs)
-	else if (id == 242 && name == "Host_Reads_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 244: Flash_Reads_32MiB (Innodisk SSDs)
-	else if (id == 244 && name == "Flash_Reads_32MiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * mib_32;
-	}
-	// Attribute 251: Total_NAND_Read_Ct_GiB (OCZ SSDs)
-	else if (id == 251 && name == "Total_NAND_Read_Ct_GiB") {
-		bytes = static_cast<uint64_t>(attr.raw_value_int) * gib;
-	}
-
-	// Set readable_value if we determined the byte count
-	if (bytes.has_value() && bytes.value() > 0) {
-		// Use binary units (KiB, MiB, GiB, TiB) for consistency with existing attributes
-		p.readable_value = hz::format_size(bytes.value(), false);
-	}
-}
-
-
 
 /// @}

src/applib/storage_property_descr_ata_attribute.h

@@ -26,11 +26,6 @@ void auto_set_ata_attribute_description(StorageProperty& p, StorageDeviceDetecte
 void storage_property_ata_attribute_autoset_warning(StorageProperty& p);
 
 
-/// Humanize SSD write statistics by converting raw values to readable byte counts.
-/// Sets the readable_value field for applicable write-related attributes.
-void storage_property_ata_attribute_humanize_ssd_writes(StorageProperty& p);
-
-
 #endif
 
 /// @}

src/applib/tests/CMakeLists.txt

@@ -15,6 +15,7 @@ endif()
 add_library(applib_tests OBJECT)
 target_sources(applib_tests PRIVATE
 	test_app_regex.cpp
+	test_selftest.cpp
 	test_smartctl_parser.cpp
 	test_smartctl_version_parser.cpp
 )

src/applib/tests/test_selftest.cpp

@@ -0,0 +1,163 @@
+/******************************************************************************
+License: BSD Zero Clause License
+Copyright:
+	(C) 2026 Alexander Shaduri <ashaduri@gmail.com>
+******************************************************************************/
+/// \file
+/// \author Alexander Shaduri
+/// \ingroup applib_tests
+/// \weakgroup applib_tests
+/// @{
+
+#include "catch2/catch.hpp"
+
+#include "applib/selftest.h"
+#include "applib/storage_device.h"
+#include <chrono>
+
+
+TEST_CASE("SelfTest basic functionality", "[selftest]")
+{
+	using namespace std::literals;
+
+	SECTION("Test type names are correct")
+	{
+		REQUIRE(SelfTest::get_test_displayable_name(SelfTest::TestType::ShortTest) != "[internal_error]");
+		REQUIRE(SelfTest::get_test_displayable_name(SelfTest::TestType::LongTest) != "[internal_error]");
+		REQUIRE(SelfTest::get_test_displayable_name(SelfTest::TestType::Conveyance) != "[internal_error]");
+	}
+
+	SECTION("Test status severity mapping")
+	{
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::Unknown) == SelfTestStatusSeverity::None);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::CompletedNoError) == SelfTestStatusSeverity::None);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::ManuallyAborted) == SelfTestStatusSeverity::Warning);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::Interrupted) == SelfTestStatusSeverity::Warning);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::CompletedWithError) == SelfTestStatusSeverity::Error);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::InProgress) == SelfTestStatusSeverity::None);
+		REQUIRE(get_self_test_status_severity(SelfTestStatus::Reserved) == SelfTestStatusSeverity::None);
+	}
+
+	SECTION("Test not active by default")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/mock");
+		SelfTest test(device, SelfTest::TestType::ShortTest);
+
+		// Test should not be active immediately after construction
+		REQUIRE(test.is_active() == false);
+		REQUIRE(test.get_status() == SelfTestStatus::Unknown);
+		REQUIRE(test.get_remaining_percent() == -1);
+	}
+
+	SECTION("Remaining seconds returns unknown when not running")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/mock");
+		SelfTest test(device, SelfTest::TestType::ShortTest);
+
+		// When no test is running, remaining seconds should be -1 (unknown)
+		REQUIRE(test.get_remaining_seconds() == -1s);
+	}
+
+	SECTION("NVMe device without duration estimate")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/nvme0");
+		device->set_detected_type(StorageDeviceDetectedType::Nvme);
+
+		SelfTest test(device, SelfTest::TestType::ShortTest);
+
+		// NVMe devices don't report duration, should return -1
+		REQUIRE(test.get_min_duration_seconds() == -1s);
+
+		// Without a running test, remaining should also be -1
+		REQUIRE(test.get_remaining_seconds() == -1s);
+	}
+
+	SECTION("Test type is correctly stored")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/mock");
+
+		SelfTest short_test(device, SelfTest::TestType::ShortTest);
+		REQUIRE(short_test.get_test_type() == SelfTest::TestType::ShortTest);
+
+		SelfTest long_test(device, SelfTest::TestType::LongTest);
+		REQUIRE(long_test.get_test_type() == SelfTest::TestType::LongTest);
+
+		SelfTest conveyance_test(device, SelfTest::TestType::Conveyance);
+		REQUIRE(conveyance_test.get_test_type() == SelfTest::TestType::Conveyance);
+	}
+
+	SECTION("Poll time is initially unknown")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/mock");
+		SelfTest test(device, SelfTest::TestType::ShortTest);
+
+		// Before starting, poll time should be -1 (unknown)
+		REQUIRE(test.get_poll_in_seconds() == -1s);
+	}
+}
+
+
+TEST_CASE("SelfTest EXT enum helpers", "[selftest][enum_helpers]")
+{
+	SECTION("Status enum to string conversion")
+	{
+		// Verify that enum helper works for common statuses
+		auto status_str = SelfTestStatusExt::get_displayable_name(SelfTestStatus::InProgress);
+		REQUIRE(!status_str.empty());
+
+		status_str = SelfTestStatusExt::get_displayable_name(SelfTestStatus::CompletedNoError);
+		REQUIRE(!status_str.empty());
+
+		status_str = SelfTestStatusExt::get_displayable_name(SelfTestStatus::Unknown);
+		REQUIRE(!status_str.empty());
+	}
+
+	SECTION("Status enum storable name")
+	{
+		// Verify storable names (for serialization/deserialization)
+		auto storable = SelfTestStatusExt::get_storable_name(SelfTestStatus::InProgress);
+		REQUIRE(storable == "in_progress");
+
+		storable = SelfTestStatusExt::get_storable_name(SelfTestStatus::ManuallyAborted);
+		REQUIRE(storable == "manually_aborted");
+
+		storable = SelfTestStatusExt::get_storable_name(SelfTestStatus::CompletedNoError);
+		REQUIRE(storable == "completed_no_error");
+	}
+
+	SECTION("Default value is Unknown")
+	{
+		REQUIRE(SelfTestStatusExt::default_value == SelfTestStatus::Unknown);
+	}
+}
+
+
+TEST_CASE("SelfTest support detection", "[selftest][support]")
+{
+	SECTION("ATA device capabilities check")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/sda");
+		device->set_detected_type(StorageDeviceDetectedType::AtaSsd);
+
+		// Without capability properties, tests should not be supported
+		SelfTest short_test(device, SelfTest::TestType::ShortTest);
+		REQUIRE(short_test.is_supported() == false);
+
+		SelfTest long_test(device, SelfTest::TestType::LongTest);
+		REQUIRE(long_test.is_supported() == false);
+	}
+
+	SECTION("NVMe conveyance test unsupported")
+	{
+		auto device = std::make_shared<StorageDevice>("/dev/nvme0");
+		device->set_detected_type(StorageDeviceDetectedType::Nvme);
+
+		// Conveyance test is not supported on NVMe
+		SelfTest conveyance_test(device, SelfTest::TestType::Conveyance);
+		REQUIRE(conveyance_test.is_supported() == false);
+	}
+}
+
+
+/// @}
+