Skip to content

Conversation

pondrejk
Copy link
Contributor

Problem Statement

In dualstack runs this caused Validation failed: Taxonomy has already been taken

Solution

Prt probably won't pass due to some other work being in progress on the ui part of the test, will try though

Related Issues

@pondrejk pondrejk self-assigned this Oct 15, 2025
@pondrejk pondrejk requested a review from a team as a code owner October 15, 2025 13:37
@pondrejk pondrejk added TestFailure Issues and PRs related to a test failing in automation CherryPick PR needs CherryPick to previous branches 6.18.z Introduced in or relating directly to Satellite 6.18 labels Oct 15, 2025
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `tests/foreman/ui/test_remoteexecution.py:223-225` </location>
<code_context>

</code_context>

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

<details><summary>Explanation</summary>Avoid complex code, like conditionals, 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_remoteexecution.py:223` </location>
<code_context>
@pytest.mark.rhel_ver_list([settings.content_host.default_rhel_version])
@pytest.mark.parametrize(
    'ui_user', [{'admin': True}, {'admin': False}], indirect=True, ids=['adminuser', 'nonadminuser']
)
def test_positive_run_custom_job_template(
    session, module_org, default_location, target_sat, ui_user, rex_contenthost
):
    """Run a job template on a host

    :id: 3a59eb15-67c4-46e1-ba5f-203496ec0b0c

    :Setup: Create a working job template.

    :steps:

        1. Set remote_execution_connect_by_ip on host to true
        2. Navigate to an individual host and click Run Job
        3. Select the job and appropriate template
        4. Run the job

    :expectedresults: Verify the job was successfully ran against the host

    :parametrized: yes

    :bz: 2220965

    :customerscenario: true
    """

    hostname = rex_contenthost.hostname
    if not any(loc.id == default_location.id for loc in ui_user.location):
        ui_user.location.append(target_sat.api.Location(id=default_location.id))
        ui_user.update(['location'])
    job_template_name = gen_string('alpha')
    with target_sat.ui_session() as session:
        session.organization.select(module_org.name)
        assert session.host.search(hostname)[0]['Name'] == hostname
        session.jobtemplate.create(
            {
                'template.name': job_template_name,
                'template.template_editor.rendering_options': 'Editor',
                'template.template_editor.editor': '<%= input("command") %>',
                'job.provider_type': 'Script',
                'inputs': [{'name': 'command', 'required': True, 'input_type': 'User input'}],
            }
        )
        assert session.jobtemplate.search(job_template_name)[0]['Name'] == job_template_name
        session.jobinvocation.run(
            {
                'category_and_template.job_category': 'Miscellaneous',
                'category_and_template.job_template_text_input': job_template_name,
                'target_hosts_and_inputs.targets': hostname,
                'target_hosts_and_inputs.command': 'ls',
            }
        )
        job_description = f'{camelize(job_template_name.lower())} with inputs command="ls"'
        session.jobinvocation.wait_job_invocation_state(
            entity_name=job_description, host_name=hostname
        )
        status = session.jobinvocation.read(entity_name=job_description, host_name=hostname)
        assert status['hosts'][0]['Status'] == 'Succeeded'

</code_context>

<issue_to_address>
**suggestion (code-quality):** Invert any/all to simplify comparisons ([`invert-any-all`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/invert-any-all/))

```suggestion
    if all(loc.id != default_location.id for loc in ui_user.location):
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

hostname = rex_contenthost.hostname
ui_user.location.append(target_sat.api.Location(id=default_location.id))
ui_user.update(['location'])
if not any(loc.id == default_location.id for loc in ui_user.location):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code-quality): Invert any/all to simplify comparisons (invert-any-all)

Suggested change
if not any(loc.id == default_location.id for loc in ui_user.location):
if all(loc.id != default_location.id for loc in ui_user.location):

@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_remoteexecution.py -k test_positive_run_custom_job_template[rhel9-ipv4-nonadminuser]

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 13192
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_remoteexecution.py -k test_positive_run_custom_job_template[rhel9-ipv4-nonadminuser] --external-logging
Test Result : ========== 1 failed, 7 deselected, 10 warnings in 1049.53s (0:17:29) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.18.z Introduced in or relating directly to Satellite 6.18 CherryPick PR needs CherryPick to previous branches PRT-Failed Indicates that latest PRT run is failed for the PR TestFailure Issues and PRs related to a test failing in automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants