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

[Repository Rewrite] Test for disabling rh repository with basearch #12777

Merged
merged 6 commits into from
Oct 17, 2023
Merged

[Repository Rewrite] Test for disabling rh repository with basearch #12777

merged 6 commits into from
Oct 17, 2023

Conversation

ColeHiggins2
Copy link
Member

Test for disabling a red hat repository with basearch set

BZ: 1932486

Steps:

  1. You have the Appstream Kicstart repositories release version "8" synced in from the release of RHEL 8
  2. hammer repository-set disable --basearch --name --product-id --organization --releasever

@ColeHiggins2 ColeHiggins2 added CherryPick PR needs CherryPick to previous branches 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 labels Sep 28, 2023
@ColeHiggins2 ColeHiggins2 self-assigned this Sep 28, 2023
@ColeHiggins2 ColeHiggins2 requested a review from a team as a code owner September 28, 2023 19:55
sambible
sambible approved these changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@sambible sambible left a comment

Choose a reason for hiding this comment

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

Looks good, just curious about that one piece

tests/foreman/cli/test_repositories.py Outdated
)
repo = module_target_sat.api.Repository(id=rh_repo_id).read()
repo.sync(timeout=2000)
module_target_sat.execute(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line just to basically assert that the repo was synced correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

to be clear, I'm referring to the repo list call after line 85

Copy link
Contributor

Choose a reason for hiding this comment

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

If so we should add an assertion

@Griffin-Sullivan
Copy link
Contributor

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

Griffin-Sullivan
Griffin-Sullivan approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@Griffin-Sullivan Griffin-Sullivan left a comment

Choose a reason for hiding this comment

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

Ack pending comment

tests/foreman/cli/test_repositories.py Outdated
)
repo = module_target_sat.api.Repository(id=rh_repo_id).read()
repo.sync(timeout=2000)
module_target_sat.execute(
Copy link
Contributor

Choose a reason for hiding this comment

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

If so we should add an assertion

@ColeHiggins2 ColeHiggins2 force-pushed the test_for_disabling_rh_repo_basearch branch from c848e9b to ea58fe4 Compare October 10, 2023 19:35
@ColeHiggins2
Copy link
Member Author

@Gauravtalreja1 @Griffin-Sullivan @sambible @shubhamsg199 Addressed comments

  1. Updated test name
  2. Removed extra sat.execute command(no longer needed)
  3. Updated to RepositorySets.disabled

Gauravtalreja1
Gauravtalreja1 reviewed Oct 11, 2023
View reviewed changes
Copy link
Collaborator

@Gauravtalreja1 Gauravtalreja1 left a comment

Choose a reason for hiding this comment

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

Ack, with a few suggestions

tests/foreman/cli/test_repositories.py Outdated Show resolved Hide resolved
tests/foreman/cli/test_repositories.py Outdated Show resolved Hide resolved
tests/foreman/cli/test_repositories.py Outdated Show resolved Hide resolved
@ColeHiggins2 ColeHiggins2 force-pushed the test_for_disabling_rh_repo_basearch branch from e971266 to 4fa36b0 Compare October 16, 2023 20:04
@ColeHiggins2
Copy link
Member Author

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

@ColeHiggins2
Copy link
Member Author

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

damoore044
damoore044 approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@damoore044 damoore044 left a comment

Choose a reason for hiding this comment

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

looks good! prt pending

@sambible sambible merged commit 55b4d0a into SatelliteQE:master Oct 17, 2023
5 checks passed
@Satellite-QE Satellite-QE mentioned this pull request Oct 17, 2023
Closed
@Satellite-QE Satellite-QE mentioned this pull request Oct 17, 2023
Closed
This was referenced Dec 12, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damoore044 damoore044 damoore044 approved these changes

@Griffin-Sullivan Griffin-Sullivan Awaiting requested review from Griffin-Sullivan

@shubhamsg199 shubhamsg199 Awaiting requested review from shubhamsg199

@sambible sambible Awaiting requested review from sambible

@Gauravtalreja1 Gauravtalreja1 Awaiting requested review from Gauravtalreja1

Assignees

@ColeHiggins2 ColeHiggins2

Labels
6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 CherryPick PR needs CherryPick to previous branches
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ColeHiggins2 @Griffin-Sullivan @sambible @Gauravtalreja1 @shubhamsg199 @damoore044