The candlepin_events service has been removed, so the service count i…

[amolpati30]

Problem Statement

The candlepin_events service was removed here, and the tests were failing due to an incorrect count.

Solution

Added a fix.

Summary by Sourcery

Tests:

• Update katello certificate check tests to expect one fewer 'ok' service in hammer ping output after candlepin_events removal.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts hammer ping expectations in katello certificate tests to account for the removal of the candlepin_events service, reducing the expected count of 'ok' services from 9 to 8.

File-Level Changes

Change	Details	Files
Update test expectations for hammer ping service count after candlepin_events removal.	Change the expected number of 'ok' occurrences in hammer ping output from 9 to 8 in the positive cert update test. Change the expected number of 'ok' occurrences in hammer ping output from 9 to 8 in the SSL regeneration/build certs test.	tests/foreman/destructive/test_katello_certs_check.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 1 issue, and left some high level feedback:

• Instead of hardcoding the expected ok count, consider asserting on the presence/absence of specific services (e.g., verifying candlepin_events is no longer listed) to make the test more resilient to future service changes.
• It may help future maintainers if you add a short code comment near the assertion explaining that the expected ok count is 8 because the candlepin_events service was removed.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Instead of hardcoding the expected `ok` count, consider asserting on the presence/absence of specific services (e.g., verifying `candlepin_events` is no longer listed) to make the test more resilient to future service changes.
- It may help future maintainers if you add a short code comment near the assertion explaining that the expected `ok` count is 8 because the `candlepin_events` service was removed.

## Individual Comments

### Comment 1
<location path="tests/foreman/destructive/test_katello_certs_check.py" line_range="56" />
<code_context>
         result = satellite.execute('hammer ping')
         assert 'SSL certificate verification failed' not in result.stdout
-        assert result.stdout.count('ok') == 9
+        assert result.stdout.count('ok') == 8
         # assert all services are running
         result = satellite.execute('satellite-maintain health check --label services-up -y')
</code_context>
<issue_to_address>
**suggestion (testing):** The strict `ok` count assertion is brittle against future service additions/removals.

This will pass for the current setup but will fail again if services change or differ between environments/plugins. Instead of hardcoding `8`, assert that the specific required services appear with `ok` (e.g., by checking expected service names and statuses) or compute the expected count from a maintained list of services used in the test.

Suggested implementation:

```python
        # assert no hammer ping SSL cert error
        result = satellite.execute('hammer ping')
        assert 'SSL certificate verification failed' not in result.stdout
        for service in EXPECTED_HAMMER_PING_SERVICES:
            # ensure each expected service is reported as ok in hammer ping
            assert service in result.stdout, f"Expected hammer ping output to contain service '{service}'"
            assert f"{service}: ok" in result.stdout or f"{service} ... ok" in result.stdout, (
                f"Expected service '{service}' to be reported as ok in hammer ping output"
            )
        # assert all services are running
        result = satellite.execute('satellite-maintain health check --label services-up -y')
        assert result.status == 0, 'Not all services are running'
    # assert no hammer ping SSL cert error
    result = target_sat.execute('hammer ping')
    assert 'SSL certificate verification failed' not in result.stdout
    for service in EXPECTED_HAMMER_PING_SERVICES:
        # ensure each expected service is reported as ok for the target satellite as well
        assert service in result.stdout, f"Expected hammer ping output to contain service '{service}' on target satellite"
        assert f"{service}: ok" in result.stdout or f"{service} ... ok" in result.stdout, (
            f"Expected service '{service}' to be reported as ok in hammer ping output on target satellite"
        )
    # assert all services are running
    result = target_sat.execute('satellite-maintain health check --label services-up -y')
    assert result.status == 0, 'Not all services are running'

```

To fully implement this change in a robust way, you will also need to:
1. Introduce an `EXPECTED_HAMMER_PING_SERVICES` iterable (e.g. list or tuple) at module scope in `tests/foreman/destructive/test_katello_certs_check.py`, containing the canonical names of the services that must be `ok` for this test to pass. Example:
   ```python
   EXPECTED_HAMMER_PING_SERVICES = (
       "candlepin",
       "candlepin_auth",
       "pulp",
       "pulp_auth",
       "foreman_tasks",
       "foreman",
       "katello",
       "katello_agent",
   )
   ```
   Adjust the actual service names to match the `hammer ping` output in your environment.
2. Confirm the exact format of each `hammer ping` line (e.g. `service: ok`, `service ... ok`, or something similar) and, if necessary, refine the two assertions so they match the real output pattern—possibly by using a regex or by splitting `result.stdout` into lines and parsing them.
3. If the test should be tolerant of extra services being present, keep `EXPECTED_HAMMER_PING_SERVICES` as the minimal required set; if it should be strict, ensure that the parsing also verifies that there are no unexpected non-ok statuses for any services.
</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.}

[amolpati30]

trigger: test-robottelo
pytest: tests/foreman/destructive/test_katello_certs_check.py -k "test_positive_update_katello_certs or test_regeneration_ssl_build_certs"

[Satellite-QE]

PRT Result

Build Number: 15190
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/destructive/test_katello_certs_check.py -k "test_positive_update_katello_certs or test_regeneration_ssl_build_certs" --external-logging
Test Result : ========== 2 passed, 3 deselected, 15 warnings in 2849.55s (0:47:29) ===========