Handle manifest upload for DEFAULT_ORG in case it was already uploaded by other test cases

[jnagare-redhat]

Problem Statement

While running this test case in pipeline, it was throwing TaskFailedError as manifest was already getting uploaded by some other test case.

Solution

• Handle manifest upload for DEFAULT_ORG in case it was already uploaded by other test cases
• Now it will not upload manifest in case its already uploaded
• Also changed module_target_sat to target_sat as per suggestion from @amolpati30

Related Issues

PRT test Cases example

trigger: test-robottelo
pytest: tests/foreman/cli/test_rhcloud_inventory.py -k test_positive_config_on_sat_without_network_protocol

[jnagare-redhat]

trigger: test-robottelo
pytest: tests/foreman/cli/test_rhcloud_inventory.py -k test_positive_config_on_sat_without_network_protocol

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• Rather than catching TaskFailedError and matching on its message, use the Satellite API to check if the manifest already exists before calling upload_manifest, so you avoid brittle string matching.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Rather than catching TaskFailedError and matching on its message, use the Satellite API to check if the manifest already exists before calling upload_manifest, so you avoid brittle string matching.

## Individual Comments

### Comment 1
<location> `tests/foreman/cli/test_rhcloud_inventory.py:811-813` </location>
<code_context>
+    status = target_sat.execute('cat /etc/systemd/system/rhcd.service.d/proxy.conf')
     # Check the correct format is present
-    assert f'Environment=NO_PROXY={module_target_sat.hostname}' in status.stdout
+    assert f'Environment=NO_PROXY={target_sat.hostname}' in status.stdout
     # Ensure NO_PROXY doesn't contain https:// prefix
     assert 'Environment=NO_PROXY=https://' not in status.stdout
</code_context>

<issue_to_address>
**suggestion (testing):** Consider parametrizing organization and manifest upload scenarios.

Parametrizing the test to cover both manifest upload states will help ensure all logic branches are tested and reduce the risk of regressions.

Suggested implementation:

```python
import pytest

@pytest.mark.parametrize(
    "organization,manifest_uploaded",
    [
        ("org1", True),
        ("org2", False),
    ]
)
def test_rhcloud_inventory_proxy_config(target_sat, organization, manifest_uploaded):
    # Setup organization and manifest upload state
    target_sat.set_organization(organization)
    if manifest_uploaded:
        target_sat.upload_manifest()
    else:
        target_sat.remove_manifest()

    # Verify the job completed successfully
    result = target_sat.api.JobInvocation(id=result.id).read()
    assert result.status_label == 'succeeded'

    # Read the rhcd service proxy configuration file to verify correct setup
    status = target_sat.execute('cat /etc/systemd/system/rhcd.service.d/proxy.conf')
    # Check the correct format is present
    assert f'Environment=NO_PROXY={target_sat.hostname}' in status.stdout
    # Ensure NO_PROXY doesn't contain https:// prefix
    assert 'Environment=NO_PROXY=https://' not in status.stdout

```

- You may need to adjust the setup logic for organization and manifest upload to match your actual API and helper methods.
- Ensure that `target_sat.set_organization`, `target_sat.upload_manifest`, and `target_sat.remove_manifest` exist or are implemented as needed.
- If the test function signature or fixture usage differs, adapt the parameterization accordingly.
</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]

@jnagare-redhat
I would recommend running the PRT with the full module to get better results with this change.

[jnagare-redhat]

pre-commit.ci autofix

[LadislavVasina1]

ACK, let's see how PRT goes.

[Satellite-QE]

PRT Result

Build Number: 13065
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_rhcloud_inventory.py -k test_positive_config_on_sat_without_network_protocol --external-logging
Test Result : ========== 1 passed, 74 deselected, 5 warnings in 1435.29s (0:23:55) ===========

[jnagare-redhat]

trigger: test-robottelo
pytest: tests/foreman/cli/test_rhcloud_inventory.py

[jnagare-redhat]

@jnagare-redhat I would recommend running the PRT with the full module to get better results with this change.

yes started full module

[Satellite-QE]

PRT Result

Build Number: 13066
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_rhcloud_inventory.py --external-logging
Test Result : === 1 failed, 63 passed, 11 deselected, 1301 warnings in 9231.00s (2:33:50) ====

[amolpati30]

I think you also need to update the module_sca_manifest.
https://github.com/SatelliteQE/robottelo/blob/master/pytest_fixtures/component/taxonomy.py#L166

[jnagare-redhat]

trigger: test-robottelo
pytest: tests/foreman/cli/test_rhcloud_inventory.py

[Satellite-QE]

PRT Result

Build Number: 13071
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_rhcloud_inventory.py --external-logging
Test Result : === 1 failed, 63 passed, 11 deselected, 1272 warnings in 9360.46s (2:36:00) ====

[jnagare-redhat]

trigger: test-robottelo
pytest: tests/foreman/cli/test_rhcloud_inventory.py

[Satellite-QE]

PRT Result

Build Number: 13081
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_rhcloud_inventory.py --external-logging
Test Result : === 1 failed, 63 passed, 11 deselected, 1382 warnings in 9549.84s (2:39:09) ====