fix(unit-tests): mock isdir so rhsm_debug tests are env agnostic

[cnsnyder]

Once we started using os.path.isdir in rhsm_debug and prior to this patch, some of our tests failed as the filesystem was being checked without looking in our test directory first.

As our unit tests were creating all the necessary and expected directories for their respective tests from within a testing directory in environments that didn't actually have /var/cache/cloud-what the tests would fail.

This fix is generic and should allow for future similar changes in the production code or in the tests to be accounted for in this way.

AI usage disclosure: The test fixes were debugged and written with AI-assistance.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Tests for rhsm_debug now monkeypatch os.path.isdir used in production code so directory existence checks are redirected to the test’s assemble_path, making the tests independent of the host filesystem layout.

File-Level Changes

Change	Details	Files
Monkeypatch os.path.isdir in rhsm_debug tests to resolve paths under the test assemble_path before checking the real filesystem, making tests environment-agnostic.	Capture the original os.path.isdir before patching so real filesystem checks remain available Define a fake_isdir helper that rewrites the checked path to live under the test assemble_path before delegating to the original isdir Use unittest.mock.patch to override rhsm_debug.debug_commands.os.path.isdir with fake_isdir during tests Register patch cleanup with addCleanup to ensure os.path.isdir is restored after tests run	test/test_rhsm_debug_command.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The fake_isdir implementation likely won't work for absolute paths (e.g. /var/cache/cloud-what) because os.path.join ignores the first argument when the second is absolute; consider normalizing to a relative path (e.g. path.lstrip(os.sep)) before joining so your test directory is actually consulted.
• Instead of manually calling self._isdir_patcher.stop() in tearDown, consider using self.addCleanup(self._isdir_patcher.stop) in setUp so the patch is reliably undone even if a test or setUp fails early.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `fake_isdir` implementation likely won't work for absolute paths (e.g. `/var/cache/cloud-what`) because `os.path.join` ignores the first argument when the second is absolute; consider normalizing to a relative path (e.g. `path.lstrip(os.sep)`) before joining so your test directory is actually consulted.
- Instead of manually calling `self._isdir_patcher.stop()` in `tearDown`, consider using `self.addCleanup(self._isdir_patcher.stop)` in `setUp` so the patch is reliably undone even if a test or `setUp` fails early.

## Individual Comments

### Comment 1
<location path="test/test_rhsm_debug_command.py" line_range="85" />
<code_context>
+                return True
+            return orig_isdir(path)
+
+        self._isdir_patcher = patch("rhsm_debug.debug_commands.os.path.isdir", side_effect=fake_isdir)
+        self._isdir_patcher.start()
+
</code_context>
<issue_to_address>
**suggestion (testing):** Use addCleanup for the os.path.isdir patch to make teardown more robust

Because the patch is started in `setUp` and stopped in `tearDown`, a failure in `setUp` (or a skipped `tearDown`) could leave the patch active and affect other tests. Register `self.addCleanup(self._isdir_patcher.stop)` immediately after starting the patch and remove the explicit `stop()` from `tearDown` so cleanup always runs, even on partial failures.

Suggested implementation:

```python
        self._isdir_patcher = patch("rhsm_debug.debug_commands.os.path.isdir", side_effect=fake_isdir)
        self._isdir_patcher.start()
        self.addCleanup(self._isdir_patcher.stop)

```

```python

```

I assumed `self._isdir_patcher.stop()` is called in `tearDown()` exactly as shown in the SEARCH block.  
If the method call is indented differently, part of another statement, or wrapped in a conditional, adjust the SEARCH pattern accordingly so that the explicit `stop()` call is fully removed from `tearDown()`.
</issue_to_address>

### Comment 2
<location path="test/test_rhsm_debug_command.py" line_range="79-80" />
<code_context>
+        # dir first, then fall back to the real filesystem.
+        orig_isdir = os.path.isdir
+
+        def fake_isdir(path):
+            test_path = path_join(self.assemble_path, path)
+            if orig_isdir(test_path):
+                return True
</code_context>
<issue_to_address>
**question (testing):** Clarify/verify behavior of fake_isdir for absolute vs relative paths

Because `os.path.join` ignores `self.assemble_path` when `path` is absolute, `orig_isdir(test_path)` is equivalent to `orig_isdir(path)` for absolute paths. That means the "prefer assemble dir" behavior only applies to relative paths. Is that consistent with how `rhsm_debug.debug_commands` constructs paths (e.g. `/var/cache/cloud-what`)? If production code passes absolute paths, these tests may still hit the host filesystem. Consider clarifying this in a comment or adjusting the helper to make the intended behavior explicit.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[github-actions]

Coverage (computed on Fedora latest) •

File	Stmts	Miss	Branch	BrPart	Cover	Missing
TOTAL	17221	4465	4744	1781	74%

report-only-changed-files is enabled. No files were changed during this commit :)

Tests	Skipped	Failures	Errors	Time
2443	14 💤	0 ❌	0 🔥	23.595s ⏱️

[DuckBoss]

LGTM, thanks!

[jirihnidek]

@sourcery-ai dismiss

[jirihnidek]

@sourcery-ai review

[SourceryAI]

Hi @jirihnidek! 👋

Only authors and team members can run @sourcery-ai commands on public repos.

If you are a team member, install the @sourcery-ai bot to get access ✨

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The fake_isdir implementation doesn’t match the comment about falling back to the real filesystem; consider checking orig_isdir(path) if orig_isdir(test_path) is false so absolute/host paths still behave as expected.
• When joining self.assemble_path and path, note that absolute paths will ignore the test directory due to os.path.join semantics; if the intent is to always prefer the test directory, you may want to normalize or strip leading slashes from path before joining.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `fake_isdir` implementation doesn’t match the comment about falling back to the real filesystem; consider checking `orig_isdir(path)` if `orig_isdir(test_path)` is false so absolute/host paths still behave as expected.
- When joining `self.assemble_path` and `path`, note that absolute paths will ignore the test directory due to `os.path.join` semantics; if the intent is to always prefer the test directory, you may want to normalize or strip leading slashes from `path` before joining.

## Individual Comments

### Comment 1
<location path="test/test_rhsm_debug_command.py" line_range="76-80" />
<code_context>

+        # Production code checks os.path.isdir() for some paths (e.g. /var/cache/cloud-what)
+        # which may not exist on the host. Patch isdir to check under the test's assemble
+        # dir first, then fall back to the real filesystem.
+        orig_isdir = os.path.isdir
+
+        def fake_isdir(path):
+            test_path = path_join(self.assemble_path, path)
+            return orig_isdir(test_path)
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The fake_isdir helper never actually falls back to the real filesystem and may not behave as intended for absolute paths.

The helper never actually "falls back" to the real filesystem: it always calls `orig_isdir(test_path)` and never `orig_isdir(path)`. For absolute paths (e.g. `/var/cache/cloud-what`), `path_join(self.assemble_path, path)` also ignores `self.assemble_path`, so those checks bypass the test directory entirely.

To align behavior with the comment and avoid surprises, consider:

```python
def fake_isdir(path):
    test_path = path_join(self.assemble_path, path.lstrip(os.sep))
    if orig_isdir(test_path):
        return True
    return orig_isdir(path)
```

Alternatively, at least special‑case absolute paths and/or update the comment to describe the actual behavior so the helper’s coupling to the host FS is clear.

Suggested implementation:

```python
from datetime import datetime
from unittest.mock import patch
import os
from os.path import join as path_join

from . import fixture
from .test_managercli import TestCliCommand
        # monkeypatch cli commands assemble path
        self.cc.assemble_path = self.assemble_path

        # Production code checks os.path.isdir() for some paths (e.g. /var/cache/cloud-what)
        # which may not exist on the host. Patch isdir to check under the test's assemble
        # dir first, then fall back to the real filesystem.
        orig_isdir = os.path.isdir

        def fake_isdir(path):
            # Check for the path under the test's assemble directory first
            test_path = path_join(self.assemble_path, path.lstrip(os.sep))
            if orig_isdir(test_path):
                return True
            # Fall back to checking the real filesystem path
            return orig_isdir(path)

```

1. Ensure that elsewhere in this test file, where `os.path.isdir` is patched, it uses `fake_isdir` (e.g. `patch("os.path.isdir", side_effect=fake_isdir)` or similar). If the existing code was directly patching `orig_isdir` or using a different helper, update those call sites to reference `fake_isdir`.
2. If there are multiple test classes or setup methods that introduce `orig_isdir`/`fake_isdir`, apply the same pattern there for consistent behavior.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}