Skip to content

Add test coverage for SAT-28860 #17153

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

Merged
jameerpathan111 merged 2 commits into from
Feb 18, 2025
Merged

Add test coverage for SAT-28860 #17153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c62e3a4
Add test coverage for SAT-28860
Gauravtalreja1 Dec 11, 2024
c72446b
Update default proxy parameter to content_default_http_proxy
Gauravtalreja1 Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions pytest_fixtures/component/http_proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,30 @@ def session_auth_proxy(session_target_sat):
@pytest.fixture
def setup_http_proxy(request, module_manifest_org, target_sat):
"""Create a new HTTP proxy and set related settings based on proxy"""
content_proxy = target_sat.api.Setting().search(
query={'search': 'name=content_default_http_proxy'}
)[0]
content_proxy_value = '' if content_proxy.value is None else content_proxy.value
general_proxy = target_sat.api.Setting().search(query={'search': 'name=http_proxy'})[0]
general_proxy_value = '' if general_proxy.value is None else general_proxy.value

http_proxy = target_sat.api_factory.make_http_proxy(module_manifest_org, request.param)
general_proxy = http_proxy.url if request.param is False else ''
if request.param:
content_proxy = target_sat.api.Setting().search(
query={'search': 'name=content_default_http_proxy'}
)[0]
assert content_proxy.value == (http_proxy.name if request.param is not None else '')

if request.param is not None:
general_proxy = (
f'http://{settings.http_proxy.username}:'
f'{settings.http_proxy.password}@{http_proxy.url[7:]}'
f'http://{settings.http_proxy.username}:{settings.http_proxy.password}@{http_proxy.url[7:]}'
if request.param
else http_proxy.url
)
content_proxy_value = target_sat.update_setting(
'content_default_http_proxy', http_proxy.name if request.param is not None else ''
)
general_proxy_value = target_sat.update_setting(
'http_proxy', general_proxy if request.param is not None else ''
)
target_sat.update_setting('http_proxy', general_proxy)
else:
target_sat.update_setting('content_default_http_proxy', '')
target_sat.update_setting('http_proxy', '')

yield http_proxy, request.param
target_sat.update_setting('content_default_http_proxy', content_proxy_value)
target_sat.update_setting('http_proxy', general_proxy_value)
Expand Down
2 changes: 2 additions & 0 deletions robottelo/host_helpers/api_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def make_http_proxy(self, org, http_proxy_type):
name=gen_string('alpha', 15),
url=settings.http_proxy.un_auth_proxy_url,
organization=[org.id],
content_default_http_proxy=True,
).create()
if http_proxy_type:
return self._satellite.api.HTTPProxy(
Expand All @@ -47,6 +48,7 @@ def make_http_proxy(self, org, http_proxy_type):
username=settings.http_proxy.username,
password=settings.http_proxy.password,
organization=[org.id],
content_default_http_proxy=True,
Copy link
Contributor

@vsedmik vsedmik Feb 17, 2025

Choose a reason for hiding this comment

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

I'm not sure if we want to always do this on proxy create via api_factory.make_http_proxy(), this fixture never done this before since that option is new. What is the reason to do so now?

Copy link
Member Author

@Gauravtalreja1 Gauravtalreja1 Feb 17, 2025

Choose a reason for hiding this comment

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

@vsedmik Yes, the fixture behaviour is still same, if we make it non-default then we had to configure the setting explicitely and that step we have removed it from this setup fixture, and this new option configures the setting on create

).create()
return None

Expand Down
57 changes: 56 additions & 1 deletion tests/foreman/ui/test_http_proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,9 @@ def test_positive_assign_http_proxy_to_products_repositories(
@pytest.mark.tier1
@pytest.mark.run_in_one_thread
@pytest.mark.parametrize('setting_update', ['content_default_http_proxy'], indirect=True)
def test_set_default_http_proxy(module_org, module_location, setting_update, target_sat):
def test_set_default_http_proxy_no_global_default(
module_org, module_location, setting_update, target_sat
):
"""Setting "Default HTTP proxy" to "no global default".

:id: e93733e1-5c05-4b7f-89e4-253b9ce55a5a
Expand Down Expand Up @@ -244,6 +246,59 @@ def test_set_default_http_proxy(module_org, module_location, setting_update, tar
assert result['table'][0]['Value'] == "Empty"


@pytest.mark.tier1
@pytest.mark.run_in_one_thread
@pytest.mark.parametrize('setting_update', ['content_default_http_proxy'], indirect=True)
def test_positive_set_default_http_proxy(
request, module_org, module_location, setting_update, target_sat
):
"""Setting "Default HTTP proxy" when new HTTP proxy is created.

:id: e93733e1-5c05-4b7f-89e4-253b9ce55a5b

:steps:
1. Navigate to Infrastructure > Http Proxies
2. Create a Http Proxy and set "Default content HTTP proxy"
3. Navigate to Administer > Settings > Content tab
4. Verify the "Default HTTP Proxy" setting with created above.
5. Update "Default HTTP Proxy" to "no global default".

:Verifies: SAT-28860

:expectedresults: Creating Http Proxy with option "Default content HTTP proxy",
updates setting "Default HTTP Proxy" succesfully.
"""
property_name = setting_update.name
http_proxy_name = gen_string('alpha', 15)
http_proxy_url = settings.http_proxy.un_auth_proxy_url

with target_sat.ui_session() as session:
session.http_proxy.create(
{
'http_proxy.name': http_proxy_name,
'http_proxy.url': http_proxy_url,
'http_proxy.content_default_http_proxy': 'true',
'locations.resources.assigned': [module_location.name],
'organizations.resources.assigned': [module_org.name],
}
)

# Teardown
@request.addfinalizer
def _finalize():
target_sat.api.HTTPProxy().search(query={'search': f'name={http_proxy_name}'})[
0
].delete()
default_proxy = target_sat.api.Setting().search(
query={'search': 'name=content_default_http_proxy'}
)[0]
assert default_proxy.value != http_proxy_name
Copy link
Contributor

@jameerpathan111 jameerpathan111 Feb 17, 2025

Choose a reason for hiding this comment

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

It's not a good practice to do assertions in teardown. Can we handle assertions in the test and keep the cleanup part separate? The results are misleading when debugging failures.

Copy link
Member Author

@Gauravtalreja1 Gauravtalreja1 Feb 17, 2025

Choose a reason for hiding this comment

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

I don't think so it'll mislead us while debugging any failures, instead I think teardowns are best to ensure the Satellite we're using is in the same state as it was before, and adding assertions there, ensures everything worked in setup and teardown

assert not default_proxy.value

result = session.settings.read(f'name = {property_name}')
assert result['table'][0]['Value'] == f'{http_proxy_name} ({http_proxy_url})'


@pytest.mark.tier1
@pytest.mark.run_in_one_thread
@pytest.mark.parametrize('setting_update', ['content_default_http_proxy'], indirect=True)
Expand Down