Use mock to avoid pre-checks for device add methods

[mulkieran]

No description provided.

[mulkieran]

/packit build

[mulkieran]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

These changes remove pragma comments from pool device operations and add tests validating that adding duplicate devices raises an error, covering both data and cache device scenarios.

Changes

Cohort / File(s)	Summary
Pragma comment removal src/stratis_cli/_actions/_pool.py	Removed trailing pragma: no cover comments from conditional blocks in add_data_devices and add_cache_devices functions; no functional changes to logic or control flow.
Test suite extension tests/integration/pool/test_add.py	Added import of StratisCliIncoherenceError; added test_add_data_again_mock_check to AddDataTestCase1 and test_add_cache_again_mock_check to AddCacheTestCase1 to verify that duplicate device additions raise StratisCliIncoherenceError when _check_same_tier is mocked to return None.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 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 mock-based tests to bypass pre-checks in device add methods to test error paths.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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: 0

🧹 Nitpick comments (1)

tests/integration/pool/test_add.py (1)

128-128: Consider using unpacking operator for list concatenation.

Static analysis suggests using the unpacking operator for better readability and performance.

🔎 Proposed refactor

Line 128:

-        command_line = self._MENU + [self._POOLNAME] + self._DEVICES
+        command_line = [*self._MENU, self._POOLNAME, *self._DEVICES]

Line 261:

-        command_line = self._MENU + [self._POOLNAME] + devices
+        command_line = [*self._MENU, self._POOLNAME, *devices]

Also applies to: 261-261

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10ed9f0 and 4bd1e23.

📒 Files selected for processing (2)

• src/stratis_cli/_actions/_pool.py
• tests/integration/pool/test_add.py

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-18T15:25:05.145Z

Learnt from: mulkieran
Repo: stratis-storage/stratis-cli PR: 0
File: :0-0
Timestamp: 2025-08-18T15:25:05.145Z
Learning: For the stratis-cli project, use Python's standard unittest framework instead of pytest. The codebase follows unittest.TestCase patterns with standard unittest assertions and test discovery.

Applied to files:

• tests/integration/pool/test_add.py

🪛 Ruff (0.14.10)

tests/integration/pool/test_add.py

128-128: Consider [*self._MENU, self._POOLNAME, *self._DEVICES] instead of concatenation

Replace with [*self._MENU, self._POOLNAME, *self._DEVICES]

(RUF005)

---

261-261: Consider [*self._MENU, self._POOLNAME, *devices] instead of concatenation

Replace with [*self._MENU, self._POOLNAME, *devices]

(RUF005)

🔇 Additional comments (4)

src/stratis_cli/_actions/_pool.py (1)

553-553: LGTM! Error paths now covered by tests.

The removal of # pragma: no cover comments is appropriate since the new mocked tests in test_add.py now exercise these error-handling paths.

Also applies to: 608-608

tests/integration/pool/test_add.py (3)

18-19: LGTM! Necessary imports for mocking tests.

The additions of patch from unittest.mock and StratisCliIncoherenceError are required for the new test cases that mock pre-checks to exercise error paths.

Also applies to: 28-28

---

121-139: LGTM! Test exercises previously uncovered error path.

The test correctly mocks _check_same_tier to bypass pre-checks, allowing the code to attempt adding duplicate devices and trigger the StratisCliIncoherenceError at line 553 in _pool.py. The use of autospec=True ensures the mock respects the function signature.

---

253-274: LGTM! Test exercises previously uncovered error path.

The test correctly mocks _check_same_tier to bypass pre-checks, allowing the code to attempt adding duplicate cache devices and trigger the StratisCliIncoherenceError at line 608 in _pool.py. The use of autospec=True ensures the mock respects the function signature.

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo dnf install -y 'dnf*-command(copr)'
• dnf copr enable packit/stratis-storage-stratis-cli-1248-copr_pull
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.