Report missing dependencies when trying to create a format

[vojtechtrefny]

Fixes: #1459

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved error messages when format creation fails due to unavailable resources. Error messages now include specific resource names and availability details instead of generic fallback text, making troubleshooting easier.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a new _format_resource property across multiple format classes to expose the underlying resource (plugin or tool) used during format creation. The error handling in ActionCreateFormat.__init__ now uses this property to provide detailed error messages including availability errors when format resources are unavailable, addressing issue #1459 regarding insufficient error context for dependencies.

Changes

Cohort / File(s)	Summary
Format Resource Property Implementations blivet/formats/__init__.py, blivet/formats/fs.py, blivet/formats/luks.py, blivet/formats/lvmpv.py, blivet/formats/mdraid.py, blivet/formats/swap.py	Added new _format_resource property to multiple format classes. Base class in DeviceFormat returns None by default; subclasses override to return their respective resources (_plugin for luks/lvmpv/mdraid/swap, _mkfs.ext for fs). No existing control flow altered.
Enhanced Error Messaging blivet/deviceaction.py	Updated ActionCreateFormat.__init__ to provide richer error messages when format is not formattable. Now includes unavailable resource name and aggregates availability errors if _format_resource is present; falls back to generic message otherwise.
Test Dependencies & Validation tests/unit_tests/devices_test/device_dependencies_test.py	Added root-privilege gate to MissingWeakDependenciesTestCase, updated plugin dependency check from "md" to "mdraid", simplified test logic by removing conditional root handling, and tightened LVM plugin unavailability error message validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding better error reporting for missing dependencies when creating formats.
Linked Issues check	✅ Passed	The PR addresses issue #1459 by exposing detailed dependency information (via _format_resource property and availability_errors) in format creation error messages.
Out of Scope Changes check	✅ Passed	All changes are scoped to enhancing format creation error reporting with dependency details as required by issue #1459.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

blivet/deviceaction.py (1)

605-612: Cache _format_resource once before message assembly.

Line 605–Line 612 repeatedly evaluate the property. Cache it locally to avoid repeated lookups and keep the block clearer.

♻️ Suggested refactor

-        if not self._format.formattable:
-            if self._format._format_resource:
-                avail_errors = ", ".join(self._format._format_resource.availability_errors)
-                msg = "resource %s to create this format %s is unavailable: %s" % (self._format._format_resource.name,
-                                                                                   self._format.type,
-                                                                                   avail_errors)
+        if not self._format.formattable:
+            resource = self._format._format_resource
+            if resource is not None:
+                avail_errors = ", ".join(resource.availability_errors)
+                msg = "resource %s to create this format %s is unavailable: %s" % (resource.name,
+                                                                                   self._format.type,
+                                                                                   avail_errors)
             else:
                 msg = "resource to create this format %s is unavailable" % self._format.type
             raise ValueError(msg)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@blivet/deviceaction.py` around lines 605 - 612, Cache
self._format._format_resource into a local variable before assembling the error
message to avoid repeated property access; e.g., assign format_resource =
self._format._format_resource, then use format_resource (and
format_resource.availability_errors and format_resource.name) when building msg,
falling back to the existing else branch that uses self._format.type as needed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit_tests/devices_test/device_dependencies_test.py`:
- Around line 198-200: The test currently passes a literal string as the regex
in assertRaisesRegex which can break if the message contains regex
metacharacters; update the assertion to pass a regex-escaped version of the
expected message (use re.escape(msg)) when calling
self.assertRaisesRegex(ValueError, ...) and add an import for the re module if
missing so the test uses re.escape(msg) to robustly match the exact message from
self.bvt.create_device(pv_fail).

---

Nitpick comments:
In `@blivet/deviceaction.py`:
- Around line 605-612: Cache self._format._format_resource into a local variable
before assembling the error message to avoid repeated property access; e.g.,
assign format_resource = self._format._format_resource, then use format_resource
(and format_resource.availability_errors and format_resource.name) when building
msg, falling back to the existing else branch that uses self._format.type as
needed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ce76d8cf-ea03-4d06-9041-e7f335221592

📥 Commits

Reviewing files that changed from the base of the PR and between 27ca797 and 0953213.

📒 Files selected for processing (8)

• blivet/deviceaction.py
• blivet/formats/__init__.py
• blivet/formats/fs.py
• blivet/formats/luks.py
• blivet/formats/lvmpv.py
• blivet/formats/mdraid.py
• blivet/formats/swap.py
• tests/unit_tests/devices_test/device_dependencies_test.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Escape the expected message when using assertRaisesRegex.

Line 198–Line 200 pass a literal string as a regex pattern; this is fragile if the message gains regex-special characters or minor formatting changes.

🧪 Suggested fix

 import os
+import re
 import unittest

-        with self.assertRaisesRegex(ValueError, msg):
+        with self.assertRaisesRegex(ValueError, re.escape(msg)):
             self.bvt.create_device(pv_fail)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit_tests/devices_test/device_dependencies_test.py` around lines 198 -
200, The test currently passes a literal string as the regex in
assertRaisesRegex which can break if the message contains regex metacharacters;
update the assertion to pass a regex-escaped version of the expected message
(use re.escape(msg)) when calling self.assertRaisesRegex(ValueError, ...) and
add an import for the re module if missing so the test uses re.escape(msg) to
robustly match the exact message from self.bvt.create_device(pv_fail).