Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RFE]Updating RHCloud tests to SCA only #15694

Merged
merged 6 commits into from
Jul 30, 2024
Merged

[RFE]Updating RHCloud tests to SCA only #15694

merged 6 commits into from
Jul 30, 2024

Conversation

ColeHiggins2
Copy link
Member

Updating rhcloud related tests to run to be SCA only.

@ColeHiggins2 ColeHiggins2 added RFE Issues that are requesting an enhancement to robottelo No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Jul 18, 2024
@ColeHiggins2 ColeHiggins2 self-assigned this Jul 18, 2024
@ColeHiggins2 ColeHiggins2 requested review from a team as code owners July 18, 2024 18:15
@ColeHiggins2
Copy link
Member Author

There doesn't seem to be many tests that use entitlements so this should be all the work.

Additionally, Should we cherry-pick this into 6.15? I am open to suggestions

jameerpathan111
jameerpathan111 reviewed Jul 19, 2024
View reviewed changes
pytest_fixtures/component/rh_cloud.py
@@ -4,10 +4,10 @@


@pytest.fixture(scope='module')
def rhcloud_manifest_org(module_target_sat, module_extra_rhel_entitlement_manifest):
Copy link
Contributor

Choose a reason for hiding this comment

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

@ColeHiggins2 should we cleanup the code and remove module_extra_rhel_entitlement_manifest fixture as it was only used by rhc tests? wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

also module_extra_rhel_entitlement_manifest_org

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should remove all of the extra_rhel fixtures for stream/6.16. There is no benefit to using more than 1 of each subscription on an SCA manifest. If the affected tests pass with SCA on earlier Satellite versions, we can cherrypick that change back as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have another jira ticket that cleans up all fixtures and code around sca. so I will do it then

@synkd
Copy link
Contributor

synkd commented Jul 19, 2024

There doesn't seem to be many tests that use entitlements so this should be all the work.

Additionally, Should we cherry-pick this into 6.15? I am open to suggestions

I would recommend running the affected tests using SCA fixtures against 6.15, 6.14, and 6.13. If they're not affected by switching to SCA, I don't see a reason not to cherrypick all the way back to 6.13.

synkd
synkd requested changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@synkd synkd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Requesting a couple of changes. Also, I see about a dozen tests that will be affected by this PR. Most of these tests are failing consistently in CI, so there may not be much value in testing them with PRT, but I would like to ensure that the passing tests continue passing with these changes. Please run a PRT job with these tests:

  • tests/foreman/cli/test_http_proxy.py::test_insights_client_registration_with_http_proxy
  • tests/foreman/ui/test_rhcloud_insights.py::test_insights_registration_with_capsule
  • tests/foreman/ui/test_rhc.py::test_positive_configure_cloud_connector

pytest_fixtures/component/rh_cloud.py Outdated
"""A module level fixture to get organization with manifest."""
org = module_target_sat.api.Organization().create()
module_target_sat.upload_manifest(org.id, module_extra_rhel_entitlement_manifest.content)
module_target_sat.upload_manifest(org.id, module_extra_rhel_sca_manifest.content)
Copy link
Contributor

Choose a reason for hiding this comment

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

module_extra_rhel_sca_manifest uses the extra_rhel_entitlement manifest category [1], so this change will not reduce the subscription footprint of the rhcloud tests. Please use module_sca_manifest with this fixture.

[1]

robottelo/pytest_fixtures/component/taxonomy.py

Line 204 in 21b5935

manifest_category=settings.manifest.extra_rhel_entitlement, simple_content_access="enabled"

pytest_fixtures/component/rh_cloud.py
@@ -4,10 +4,10 @@


@pytest.fixture(scope='module')
def rhcloud_manifest_org(module_target_sat, module_extra_rhel_entitlement_manifest):
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should remove all of the extra_rhel fixtures for stream/6.16. There is no benefit to using more than 1 of each subscription on an SCA manifest. If the affected tests pass with SCA on earlier Satellite versions, we can cherrypick that change back as well.

@ColeHiggins2
Copy link
Member Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_rhcloud_insights.py tests/foreman/ui/test_rhc.py tests/foreman/cli/test_http_proxy.py -k 'test_insights_client_registration_with_http_proxy or test_insights_registration_with_capsule or test_positive_configure_cloud_connector'

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7881
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_rhcloud_insights.py tests/foreman/ui/test_rhc.py tests/foreman/cli/test_http_proxy.py -k test_insights_client_registration_with_http_proxy or test_insights_registration_with_capsule or test_positive_configure_cloud_connector --external-logging
Test Result : ======== 13 passed, 15 deselected, 1572 warnings in 10469.36s (2:54:29) ========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 26, 2024
@pondrejk pondrejk merged commit 19eac0e into SatelliteQE:master Jul 30, 2024
10 checks passed
jyejare pushed a commit to jyejare/robottelo that referenced this pull request Oct 19, 2024
@ColeHiggins2 @jyejare
[RFE]Updating RHCloud tests to SCA only (SatelliteQE#15694)
9a882e1 
* Adding pit marker for tests

* Updating tests for rh cloud sca only

* rh cloud sca only pick

* rhcloud sca only pick2

* Addressing comments for sca org

* removing cherrypicked Pr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jameerpathan111 jameerpathan111 jameerpathan111 approved these changes

@pondrejk pondrejk pondrejk approved these changes

@vijaysawant vijaysawant vijaysawant approved these changes

@vsedmik vsedmik vsedmik approved these changes

@synkd synkd synkd approved these changes

Assignees

@ColeHiggins2 ColeHiggins2

Labels
No-CherryPick PR doesnt need CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR RFE Issues that are requesting an enhancement to robottelo Stream Introduced in or relating directly to Satellite Stream/Master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ColeHiggins2 @synkd @Satellite-QE @pondrejk @jameerpathan111 @vijaysawant @vsedmik