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.

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

fix tests from repositories component #16227

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

vijaysawant
Copy link
Contributor

@vijaysawant vijaysawant commented Sep 5, 2024

Problem Statement

[6.16.0] TFA - Compoenent repositories has few tests failing with minor issue, DISTRIBUTOR_CONFLICT , Assertion Error, etc
api/test_product.py::test_positive_filter_product_list
api/test_repositories.py::test_negative_upload_expired_manifest
api/test_repositories.py::test_positive_sync_mulitple_large_repos

Solution

This PR will address those failures

Related Issues

N/A

PRT test Cases example

trigger: test-robottelo
pytest: tests/foreman/api/test_product.py::test_positive_filter_product_list
trigger: test-robottelo
pytest: tests/foreman/api/test_repositories.py::test_negative_upload_expired_manifest
trigger: test-robottelo
pytest: tests/foreman/api/test_repositories.py::test_positive_sync_mulitple_large_repos

@vijaysawant vijaysawant added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing Stream Introduced in or relating directly to Satellite Stream/Master 6.16.z labels Sep 5, 2024
@vijaysawant
Copy link
Contributor Author

vijaysawant commented Sep 5, 2024

trigger: test-robottelo
pytest: tests/foreman/api/test_product.py::test_positive_filter_product_list

1 similar comment
@vijaysawant
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_product.py::test_positive_filter_product_list

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8498
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_product.py::test_positive_filter_product_list --external-logging
Test Result : ================== 1 passed, 9 warnings in 2361.58s (0:39:21) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 6, 2024
@vijaysawant
Copy link
Contributor Author

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

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8520
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_repositories.py --external-logging
Test Result : ====== 2 failed, 10 passed, 1 skipped, 330 warnings in 4112.81s (1:08:32) ======

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Sep 7, 2024
@vijaysawant
Copy link
Contributor Author

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

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8626
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_repositories.py --external-logging
Test Result : =========== 12 passed, 1 skipped, 360 warnings in 1787.15s (0:29:47) ===========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Sep 12, 2024
@vijaysawant vijaysawant marked this pull request as ready for review September 13, 2024 02:24
@vijaysawant vijaysawant requested a review from a team as a code owner September 13, 2024 02:24
@@ -209,8 +209,8 @@ def test_positive_filter_product_list(module_sca_manifest_org, module_target_sat
query={'redhat_only': True, 'per_page': 1000}
)

assert len(custom_products) == 1
assert product.name == custom_products[0].name
assert len(custom_products) >= 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know what else is returned by the search on L207 and why? Is it another custom product created by another test within the same org?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when we run whole test module, at L207 we get list of more than one custom products, so at this assertion L212, using greater than one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then the assertion doesn't make much sense to me. It just asserts that one of the test in the module (no matter which one) created some custom product.
We can go with function-scoped org and assert == 1 or remove this assertion and rely on the next one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, removing this assertion, if we use function scoped org then another assertion need to adjust (len of rh_products > 1)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes agree with @vsedmik , we could scope by function_org and should only have the one created,
or you should able to narrow the search of custom products in module_org, by the name/id/label of the one created just prior?
Do you ~need/expect multiple custom products, or is this just handling when multiple are present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@damoore044 If we look at L207, custom_product is being search using organization, another test create custom product hence it is giving more than one custom product.
I would remove the assertion and will rely on next one.

tests/foreman/api/test_repositories.py Outdated Show resolved Hide resolved
@vijaysawant
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/api/test_product.py::test_positive_filter_product_list

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 13, 2024
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.

Ack nice!
pending prior comments and nonblocking thoughts on the first assertion.

@@ -209,8 +209,8 @@ def test_positive_filter_product_list(module_sca_manifest_org, module_target_sat
query={'redhat_only': True, 'per_page': 1000}
)

assert len(custom_products) == 1
assert product.name == custom_products[0].name
assert len(custom_products) >= 1
Copy link
Contributor

Choose a reason for hiding this comment

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

yes agree with @vsedmik , we could scope by function_org and should only have the one created,
or you should able to narrow the search of custom products in module_org, by the name/id/label of the one created just prior?
Do you ~need/expect multiple custom products, or is this just handling when multiple are present?

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8643
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_product.py::test_positive_filter_product_list --external-logging
Test Result : ================== 1 passed, 9 warnings in 791.08s (0:13:11) ===================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 13, 2024
@vijaysawant
Copy link
Contributor Author

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

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 13, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8646
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_repositories.py --external-logging
Test Result : =========== 12 passed, 1 skipped, 348 warnings in 1765.10s (0:29:25) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 13, 2024
@vijaysawant
Copy link
Contributor Author

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

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8647
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_repository.py --external-logging
Test Result : ===== 1 failed, 111 passed, 5 skipped, 483 warnings in 2143.88s (0:35:43) ======

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Sep 13, 2024
@vijaysawant
Copy link
Contributor Author

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

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8700
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_repositories.py --external-logging
Test Result : ================ 13 passed, 375 warnings in 1920.63s (0:32:00) =================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Sep 18, 2024
Copy link
Contributor

@vsedmik vsedmik left a comment

Choose a reason for hiding this comment

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

LGTM. Is the do_not_merge label still valid @vijaysawant ?

@vsedmik vsedmik merged commit 14efc53 into SatelliteQE:master Sep 18, 2024
21 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.16.z AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR Stream Introduced in or relating directly to Satellite Stream/Master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants