New All Hosts Features - Manage HostCollections, Host Status coverage

[LadislavVasina1]

Problem statement

Provide coverage for new 6.19 features,
which are part of EPIC [Part 4] - New All Hosts page - Complete the top actions and eliminate some AngularJS pages

• SAT-31027 (As a user I want to be able to change the host collection of multiple hosts)
• SAT-36330 (As a user I would like to see the overall status of each host in the list)

Solution

Addition of new tests test_positive_all_hosts_manage_host_collections and test_positive_all_hosts_check_status_icon.

Additional info

HostCollection changes are not in a stream yet.

Needs:
theforeman/foreman#10721
Katello/katello#11528
SatelliteQE/airgun#2155

PRT Example

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py -k "test_positive_all_hosts_manage_host_collections or test_positive_all_hosts_check_status_icon"
airgun: 2155
theforeman:
    foreman: 10721
Katello:
    katello: 11528

[sourcery-ai]

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

• Consider moving read_host_collections_hosts helper into a shared testing utility module to avoid cluttering the test file and promote reuse.
• Split the monolithic test_positive_all_hosts_manage_host_collections into smaller, focused tests or parameterize the add/remove scenarios to improve readability and maintenance.
• Extract the repeated UI session setup and tear-down logic into a fixture or helper to reduce boilerplate and make tests more concise.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider moving read_host_collections_hosts helper into a shared testing utility module to avoid cluttering the test file and promote reuse.
- Split the monolithic test_positive_all_hosts_manage_host_collections into smaller, focused tests or parameterize the add/remove scenarios to improve readability and maintenance.
- Extract the repeated UI session setup and tear-down logic into a fixture or helper to reduce boilerplate and make tests more concise.

## Individual Comments

### Comment 1
<location> `tests/foreman/ui/test_host.py:3216-3223` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))

<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.

Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals

Some ways to fix this:

* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.

> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>

### Comment 2
<location> `tests/foreman/ui/test_host.py:3228-3233` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))

<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.

Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals

Some ways to fix this:

* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.

> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>

### Comment 3
<location> `tests/foreman/ui/test_host.py:3175-3178` </location>
<code_context>
def read_host_collections_hosts(target_sat, host_col_names, org):
    """
    Helper function to read hosts in a host collections via API.

    :param target_sat: Satellite object for API access
    :param host_col_names: list of host collection names or a single host collection name
    :param org: Organization object the host collections belong to

    :return: dict mapping host collection names to lists of host names in each collection
    """

    if not isinstance(host_col_names, list):
        host_col_names = [host_col_names]

    hosts_assigned_to_collections = {}
    for host_col in host_col_names:
        host_collection = target_sat.api.HostCollection(organization=org).search(
            query={'search': f'name="{host_col}"'}
        )
        if host_collection:
            hc = host_collection[0].read()
            hosts_assigned_to_collections[host_col] = [host.read().name for host in hc.host]
        else:
            hosts_assigned_to_collections[host_col] = []

    return hosts_assigned_to_collections

</code_context>

<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))

```suggestion
        if host_collection := target_sat.api.HostCollection(
            organization=org
        ).search(query={'search': f'name="{host_col}"'}):
```
</issue_to_address>

### Comment 4
<location> `tests/foreman/ui/test_host.py:3278` </location>
<code_context>
def test_positive_all_hosts_manage_host_collections(target_sat, function_org, function_location):
    """
    Manage host collections for multiple hosts through All Hosts UI bulk actions.

    :id: ef9d1521-7cf1-4d86-a5b4-197f34fcf907

    :steps:
        1. Create 2 fake hosts
        2. Create 2 host collections with unlimited host limits
        3. Create 1 host collection with host limit set to 1
        4. Add 1 host to the host collection with limit=1 (should succeed)
        5. Attempt to add a second host to the host collection with limit=1 (should fail)
        6. Add both hosts to the 2 unlimited host collections (should succeed)
        7. Remove 1 host from 1 host collection
        8. Remove 1 host from 2 host collections simultaneously
        9. Add hosts back to both collections
        10. Remove all hosts from all collections

    :expectedresults:
        1. Hosts can be successfully added to host collections via bulk action
        2. Host collection limits are enforced (limit=1 prevents adding second host)
        3. Multiple hosts can be added to multiple host collections in one operation
        4. Hosts can be removed from single or multiple host collections
        5. Success/failure messages are displayed appropriately
        6. Host collection membership is correctly updated after each operation
        7. API verification confirms the UI changes match actual host collection membership
    """

    host_names = []
    for _ in range(2):
        host = target_sat.cli_factory.make_fake_host(
            {
                'organization': function_org.name,
                'location': function_location.name,
            }
        )
        host_names.append(host.name)

    host_col_names = ['TestHostCol_1', 'TestHostCol_2']

    # create two host collections with unlimited hosts limit
    for col_name in host_col_names:
        target_sat.api.HostCollection(
            name=col_name,
            max_hosts=None,
            organization=function_org,
        ).create()

    # create one host collection with host_limit=1
    host_col_limit_1_name = 'TestHostCol_limit_1'
    target_sat.api.HostCollection(
        name=host_col_limit_1_name,
        max_hosts=1,
        organization=function_org,
    ).create()

    with target_sat.ui_session() as session:
        session.organization.select(org_name=function_org.name)
        session.location.select(loc_name=function_location.name)

        # Add 1 host to host_collection with limit=1 - should succeed
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names[0],
            host_collections_to_select=host_col_limit_1_name,
            option='Add',
        )
        assert 'Successfully added' in result
        hosts_in_host_col = read_host_collections_hosts(
            target_sat, host_col_limit_1_name, function_org
        )
        assert hosts_in_host_col[host_col_limit_1_name] == [host_names[0]]

        # Add second host to host_collection with limit=1 - should fail
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names[1],
            host_collections_to_select=host_col_limit_1_name,
            option='Add',
        )
        assert 'Validation failed' in result
        hosts_in_host_col = read_host_collections_hosts(
            target_sat, host_col_limit_1_name, function_org
        )
        assert hosts_in_host_col[host_col_limit_1_name] == [host_names[0]]

        # Add 2 hosts to 2 host collections
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names,
            host_collections_to_select=host_col_names,
            option='Add',
        )
        assert 'Successfully added' in result
        hosts_in_host_col = read_host_collections_hosts(target_sat, host_col_names, function_org)
        assert hosts_in_host_col[host_col_names[0]] == host_names
        assert hosts_in_host_col[host_col_names[1]] == host_names

        # Remove 1 host from 1 host collection
        # at this state there are 2 hosts in both 'TestHostCol_1' and 'TestHostCol_2'
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names[0],
            host_collections_to_select=host_col_names[0],
            option='Remove',
        )
        assert 'Successfully removed' in result
        hosts_in_host_col = read_host_collections_hosts(target_sat, host_col_names[0], function_org)
        assert hosts_in_host_col[host_col_names[0]] == [host_names[1]]

        # Remove 1 host from 2 host collections
        # at this state there is 1 host in 'TestHostCol_1' and 2 hosts in 'TestHostCol_2'
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names[1],
            host_collections_to_select=host_col_names,
            option='Remove',
        )
        assert 'Successfully removed' in result
        hosts_in_host_col = read_host_collections_hosts(target_sat, host_col_names, function_org)
        assert hosts_in_host_col[host_col_names[0]] == []
        assert hosts_in_host_col[host_col_names[1]] == [host_names[0]]

        # Add hosts back, so both host collections have 2 hosts
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names,
            host_collections_to_select=host_col_names,
            option='Add',
        )
        hosts_in_host_col = read_host_collections_hosts(target_sat, host_col_names, function_org)
        assert hosts_in_host_col[host_col_names[0]] == host_names
        assert hosts_in_host_col[host_col_names[1]] == host_names

        # Remove 2 hosts from 2 host collections
        result = session.all_hosts.change_associations_host_collections(
            host_names=host_names,
            host_collections_to_select=host_col_names,
            option='Remove',
        )
        hosts_in_host_col = read_host_collections_hosts(target_sat, host_col_names, function_org)
        assert hosts_in_host_col[host_col_names[0]] == []
        assert hosts_in_host_col[host_col_names[1]] == []

</code_context>

<issue_to_address>
**issue (code-quality):** Extract duplicate code into function ([`extract-duplicate-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-duplicate-method/))
</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.}

[LadislavVasina1]

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py -k "test_positive_all_hosts_manage_host_collections or test_positive_all_hosts_check_status_icon"
airgun: 2155
theforeman:
    foreman: 10721
Katello:
    katello: 11528

[synkd]

Ack pending requested changes.

[LadislavVasina1]

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py -k "test_positive_all_hosts_manage_host_collections or test_positive_all_hosts_check_status_icon"
airgun: 2155
theforeman:
    foreman: 10721
Katello:
    katello: 11528