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

Workaround the issue as long as BZ2228820 is open #12498

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Workaround the issue as long as BZ2228820 is open #12498

merged 1 commit into from
Sep 11, 2023

Conversation

lhellebr
Copy link
Contributor

@lhellebr lhellebr commented Sep 5, 2023

No description provided.

@lhellebr lhellebr requested a review from a team as a code owner September 5, 2023 15:20
@lhellebr lhellebr 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 5, 2023
ColeHiggins2
ColeHiggins2 approved these changes Sep 5, 2023
View reviewed changes
Copy link
Member

@ColeHiggins2 ColeHiggins2 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.

Looks good to me. waiting for PRT job

@ColeHiggins2
Copy link
Member

trigger: test-robottelo
pytest: tests/foreman/api/test_organization.py -k test_positive_create

Gauravtalreja1
Gauravtalreja1 reviewed Sep 5, 2023
View reviewed changes
tests/foreman/api/test_organization.py
@@ -77,7 +78,10 @@ def test_positive_create(self):
headers={'content-type': 'text/plain'},
Copy link
Collaborator

Choose a reason for hiding this comment

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

For workaround here, wouldn't it better to change content-type instead when bz is open with is_open and not checking UNSUPPORTED_MEDIA_TYPE, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it is now, we're testing that UNSUPPORTED_MEDIA_TYPE is being returned, or at least ISE 500 if the BZ is open (as opposed to returning something even more wrong, e.g. 200 OK).

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO using any other content-type's isn't wrong, and I feel its better option to test something than checking if we hit UNSUPPORTED_MEDIA_TYPE/ISE :)

Its upto you how you want to test it, so its non-blocking.

@lhellebr lhellebr requested a review from a team September 6, 2023 11:51
@Gauravtalreja1 Gauravtalreja1 added TestFailure Issues and PRs related to a test failing in automation AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing labels Sep 6, 2023
@lhellebr lhellebr requested review from a team September 7, 2023 06:55
@Gauravtalreja1
Copy link
Collaborator

Why you're looking for additional tier1 and tier2 reviews on this? :)

@adarshdubey-star
Copy link
Contributor

Please add the description and we can merge this!

@lhellebr
Copy link
Contributor Author

@Gauravtalreja1, because nobody merged and I shouldn't self-merge.

@lhellebr
Copy link
Contributor Author

@adarshdubey-star there's both PR name and commit message, why do you think description is necessary? Is something unclear based on the commit message?

@Gauravtalreja1 Gauravtalreja1 merged commit c8dce95 into SatelliteQE:master Sep 11, 2023
@Satellite-QE Satellite-QE mentioned this pull request Sep 11, 2023
Merged
github-actions bot pushed a commit that referenced this pull request Sep 11, 2023
@lhellebr @web-flow
Workaround the issue as long as BZ2228820 is open (#12498)
038aec1 
(cherry picked from commit c8dce95)
github-actions bot pushed a commit that referenced this pull request Sep 11, 2023
@lhellebr @web-flow
Workaround the issue as long as BZ2228820 is open (#12498)
ccba102 
(cherry picked from commit c8dce95)
@Satellite-QE Satellite-QE mentioned this pull request Sep 11, 2023
Merged
@adarshdubey-star
Copy link
Contributor

@adarshdubey-star there's both PR name and commit message, why do you think description is necessary? Is something unclear based on the commit message?

Nope just a good to have thing :)

vsedmik pushed a commit to vsedmik/robottelo that referenced this pull request Sep 19, 2023
@lhellebr @vsedmik
Workaround the issue as long as BZ2228820 is open (SatelliteQE#12498)
5e26ade
ColeHiggins2 pushed a commit to ColeHiggins2/robottelo that referenced this pull request Oct 9, 2023
@lhellebr @ColeHiggins2
Workaround the issue as long as BZ2228820 is open (SatelliteQE#12498)
185d8d4
damoore044 pushed a commit to damoore044/robottelo that referenced this pull request Oct 10, 2023
@lhellebr @damoore044
Workaround the issue as long as BZ2228820 is open (SatelliteQE#12498)
04344a8
shweta83 pushed a commit to shweta83/robottelo that referenced this pull request Apr 10, 2024
@lhellebr @shweta83
Workaround the issue as long as BZ2228820 is open (SatelliteQE#12498)
ce1bbbc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 approved these changes

@ColeHiggins2 ColeHiggins2 ColeHiggins2 approved these changes

Assignees

@lhellebr lhellebr

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 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches TestFailure Issues and PRs related to a test failing in automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lhellebr @ColeHiggins2 @Gauravtalreja1 @adarshdubey-star