[6.15.z] addFinalizer in test body (#14422)

addFinalizer in test body (#14036)

* reordered addFinalizer in test body

* Update tests/foreman/cli/test_errata.py

Co-authored-by: Gaurav Talreja <gauravtalreja1@gmail.com>

* Update tests/foreman/api/test_discoveryrule.py

Co-authored-by: Gaurav Talreja <gauravtalreja1@gmail.com>

* Update tests/foreman/cli/test_errata.py

Co-authored-by: Gaurav Talreja <gauravtalreja1@gmail.com>

* Update tests/foreman/destructive/test_capsule_loadbalancer.py

Co-authored-by: Gaurav Talreja <gauravtalreja1@gmail.com>

---------

Co-authored-by: Gaurav Talreja <gauravtalreja1@gmail.com>
(cherry picked from commit 84e54f2)

Co-authored-by: Peter Ondrejka <pondrejk@users.noreply.github.com>

tests/foreman/api/test_computeresource_libvirt.py

@@ -113,9 +113,9 @@ def test_positive_create_with_name_description(
         location=[module_location],
         url=LIBVIRT_URL,
     ).create()
+    request.addfinalizer(compresource.delete)
     assert compresource.name == name
     assert compresource.description == name
-    request.addfinalizer(compresource.delete)
 
 
 @pytest.mark.tier2
@@ -134,9 +134,9 @@ def test_positive_create_with_orgs_and_locs(request, module_target_sat):
     compresource = module_target_sat.api.LibvirtComputeResource(
         location=locs, organization=orgs, url=LIBVIRT_URL
     ).create()
+    request.addfinalizer(compresource.delete)
     assert {org.name for org in orgs} == {org.read().name for org in compresource.organization}
     assert {loc.name for loc in locs} == {loc.read().name for loc in compresource.location}
-    request.addfinalizer(compresource.delete)
 
 
 @pytest.mark.tier2
@@ -175,8 +175,8 @@ def test_negative_create_with_same_name(request, module_target_sat, module_org,
     cr = module_target_sat.api.LibvirtComputeResource(
         location=[module_location], name=name, organization=[module_org], url=LIBVIRT_URL
     ).create()
-    assert cr.name == name
     request.addfinalizer(cr.delete)
+    assert cr.name == name
     with pytest.raises(HTTPError):
         module_target_sat.api.LibvirtComputeResource(
             name=name,
@@ -245,19 +245,16 @@ def test_negative_update_same_name(request, module_target_sat, module_org, modul
     compresource = module_target_sat.api.LibvirtComputeResource(
         location=[module_location], name=name, organization=[module_org], url=LIBVIRT_URL
     ).create()
+    request.addfinalizer(compresource.delete)
     new_compresource = module_target_sat.api.LibvirtComputeResource(
         location=[module_location], organization=[module_org], url=LIBVIRT_URL
     ).create()
+    request.addfinalizer(new_compresource.delete)
     new_compresource.name = name
     with pytest.raises(HTTPError):
         new_compresource.update(['name'])
     assert new_compresource.read().name != name
 
-    @request.addfinalizer
-    def _finalize():
-        compresource.delete()
-        new_compresource.delete()
-
 
 @pytest.mark.tier2
 @pytest.mark.parametrize('url', **parametrized({'random': gen_string('alpha'), 'empty': ''}))

tests/foreman/api/test_discoveryrule.py

@@ -171,8 +171,11 @@ def test_positive_multi_provision_with_rule_limit(
 
     :CaseImportance: High
     """
+
     discovered_host1 = module_target_sat.api_factory.create_discovered_host()
+    request.addfinalizer(module_target_sat.api.Host(id=discovered_host1['id']).delete)
     discovered_host2 = module_target_sat.api_factory.create_discovered_host()
+    request.addfinalizer(module_target_sat.api.DiscoveredHost(id=discovered_host2['id']).delete)
     rule = module_target_sat.api.DiscoveryRule(
         max_count=1,
         hostgroup=module_discovery_hostgroup,
@@ -181,14 +184,6 @@ def test_positive_multi_provision_with_rule_limit(
         organization=[discovery_org],
         priority=1000,
     ).create()
+    request.addfinalizer(rule.delete)
     result = module_target_sat.api.DiscoveredHost().auto_provision_all()
     assert '1 discovered hosts were provisioned' in result['message']
-
-    # Delete discovery rule
-    @request.addfinalizer
-    def _finalize():
-        rule.delete()
-        module_target_sat.api.Host(id=discovered_host1['id']).delete()
-        module_target_sat.api.DiscoveredHost(id=discovered_host2['id']).delete()
-        with pytest.raises(HTTPError):
-            rule.read()

tests/foreman/api/test_http_proxy.py

@@ -295,6 +295,11 @@ def test_positive_sync_proxy_with_certificate(request, target_sat, module_org, m
 
     :customerscenario: true
     """
+
+    @request.addfinalizer
+    def _finalize():
+        target_sat.custom_certs_cleanup()
+
     # Cleanup any existing certs that may conflict
     target_sat.custom_certs_cleanup()
     proxy_host = settings.http_proxy.auth_proxy_url.replace('http://', '').replace(':3128', '')

tests/foreman/api/test_parameters.py

@@ -39,7 +39,9 @@ def test_positive_parameter_precedence_impact(
     param_value = gen_string('alpha')
 
     cp = module_target_sat.api.CommonParameter(name=param_name, value=param_value).create()
+    request.addfinalizer(cp.delete)
     host = module_target_sat.api.Host(organization=module_org, location=module_location).create()
+    request.addfinalizer(host.delete)
     result = [res for res in host.all_parameters if res['name'] == param_name]
     assert result[0]['name'] == param_name
     assert result[0]['associated_type'] == 'global'
@@ -48,19 +50,14 @@ def test_positive_parameter_precedence_impact(
         organization=[module_org],
         group_parameters_attributes=[{'name': param_name, 'value': param_value}],
     ).create()
+    request.addfinalizer(hg.delete)
     host.hostgroup = hg
     host = host.update(['hostgroup'])
     result = [res for res in host.all_parameters if res['name'] == param_name]
     assert result[0]['name'] == param_name
     assert result[0]['associated_type'] != 'global'
     assert result[0]['associated_type'] == 'host group'
 
-    @request.addfinalizer
-    def _finalize():
-        host.delete()
-        hg.delete()
-        cp.delete()
-
     host.host_parameters_attributes = [{'name': param_name, 'value': param_value}]
     host = host.update(['host_parameters_attributes'])
     result = [res for res in host.all_parameters if res['name'] == param_name]

tests/foreman/cli/test_classparameters.py

@@ -86,8 +86,8 @@ def test_positive_list(
             location=module_puppet_loc.id,
             environment=module_puppet['env'].name,
         ).create()
-        host.add_puppetclass(data={'puppetclass_id': module_puppet['class']['id']})
         request.addfinalizer(host.delete)
+        host.add_puppetclass(data={'puppetclass_id': module_puppet['class']['id']})
         hostgroup = session_puppet_enabled_sat.cli_factory.hostgroup(
             {
                 'puppet-environment-id': module_puppet['env'].id,

tests/foreman/cli/test_computeresource_osp.py

@@ -76,6 +76,7 @@ def test_crud_and_duplicate_name(self, request, id_type, osp_version, target_sat
                 'url': osp_version,
             }
         )
+        request.addfinalizer(lambda: self.cr_cleanup(compute_resource['id'], id_type, target_sat))
         assert compute_resource['name'] == name
         assert target_sat.cli.ComputeResource.exists(search=(id_type, compute_resource[id_type]))
 
@@ -102,7 +103,6 @@ def test_crud_and_duplicate_name(self, request, id_type, osp_version, target_sat
         else:
             compute_resource = target_sat.cli.ComputeResource.info({'id': compute_resource['id']})
         assert new_name == compute_resource['name']
-        request.addfinalizer(lambda: self.cr_cleanup(compute_resource['id'], id_type, target_sat))
 
     @pytest.mark.tier3
     def test_negative_create_osp_with_url(self, target_sat):

tests/foreman/cli/test_discoveredhost.py

@@ -70,10 +70,8 @@ def test_rhel_pxe_discovery_provisioning(
 
     assert 'Host created' in result[0]['message']
     host = sat.api.Host().search(query={"search": f'id={discovered_host.id}'})[0]
-    assert host
-
-    # teardown
     request.addfinalizer(lambda: sat.provisioning_cleanup(host.name))
+    assert host
 
     wait_for(
         lambda: host.read().build_status_label != 'Pending installation',
@@ -131,10 +129,8 @@ def test_rhel_pxeless_discovery_provisioning(
     )
     assert 'Host created' in result[0]['message']
     host = sat.api.Host().search(query={"search": f'id={discovered_host.id}'})[0]
-    assert host
-
-    # teardown
     request.addfinalizer(lambda: sat.provisioning_cleanup(host.name))
+    assert host
 
     wait_for(
         lambda: host.read().build_status_label != 'Pending installation',

tests/foreman/cli/test_errata.py

@@ -688,16 +688,11 @@ def test_positive_list_affected_chosts_by_erratum_restrict_flag(
             'inclusion': 'false',
         }
     )
-
-    @request.addfinalizer
-    def cleanup():
-        cv_filter_cleanup(
-            target_sat,
-            cv_filter['filter-id'],
-            module_cv,
-            module_sca_manifest_org,
-            module_lce,
+    request.addfinalizer(
+        lambda: cv_filter_cleanup(
+            target_sat, cv_filter['filter-id'], module_cv, module_sca_manifest_org, module_lce
         )
+    )
 
     # Make rule to hide the RPM that creates the need for the installable erratum
     target_sat.cli_factory.content_view_filter_rule(
@@ -861,17 +856,11 @@ def test_host_errata_search_commands(
             'inclusion': 'false',
         }
     )
-
-    @request.addfinalizer
-    def cleanup():
-        cv_filter_cleanup(
-            target_sat,
-            cv_filter['filter-id'],
-            module_cv,
-            module_sca_manifest_org,
-            module_lce,
+    request.addfinalizer(
+        lambda: cv_filter_cleanup(
+            target_sat, cv_filter['filter-id'], module_cv, module_sca_manifest_org, module_lce
         )
-
+    )
     # Make rule to exclude the specified bugfix package
     target_sat.cli_factory.content_view_filter_rule(
         {

tests/foreman/cli/test_hammer.py

@@ -136,6 +136,13 @@ def test_positive_disable_hammer_defaults(request, function_product, target_sat)
 
     :BZ: 1640644, 1368173
     """
+
+    @request.addfinalizer
+    def _finalize():
+        target_sat.cli.Defaults.delete({'param-name': 'organization_id'})
+        result = target_sat.execute('hammer defaults list')
+        assert str(function_product.organization.id) not in result.stdout
+
     target_sat.cli.Defaults.add(
         {'param-name': 'organization_id', 'param-value': function_product.organization.id}
     )
@@ -154,12 +161,6 @@ def test_positive_disable_hammer_defaults(request, function_product, target_sat)
     assert result.status == 0
     assert function_product.name in result.stdout
 
-    @request.addfinalizer
-    def _finalize():
-        target_sat.cli.Defaults.delete({'param-name': 'organization_id'})
-        result = target_sat.execute('hammer defaults list')
-        assert str(function_product.organization.id) not in result.stdout
-
 
 @pytest.mark.upgrade
 def test_positive_check_debug_log_levels(target_sat):

tests/foreman/cli/test_satellitesync.py

@@ -2272,6 +2272,7 @@ def test_positive_custom_cdn_with_credential(
         meta_file = 'metadata.json'
         crt_file = 'source.crt'
         pub_dir = '/var/www/html/pub/repos'
+        request.addfinalizer(lambda: target_sat.execute(f'rm -rf {pub_dir}'))
 
         # Export the repository in syncable format and move it
         # to /var/www/html/pub/repos to mimic custom CDN.
@@ -2288,7 +2289,6 @@ def test_positive_custom_cdn_with_credential(
         exp_dir = exp_dir[0].replace(meta_file, '')
 
         assert target_sat.execute(f'mv {exp_dir} {pub_dir}').status == 0
-        request.addfinalizer(lambda: target_sat.execute(f'rm -rf {pub_dir}'))
         target_sat.execute(f'semanage fcontext -a -t httpd_sys_content_t "{pub_dir}(/.*)?"')
         target_sat.execute(f'restorecon -R {pub_dir}')
 

tests/foreman/cli/test_subnet.py

@@ -199,8 +199,8 @@ def test_negative_update_attributes(request, options, module_target_sat):
     :CaseImportance: Medium
     """
     subnet = module_target_sat.cli_factory.make_subnet()
-    options['id'] = subnet['id']
     request.addfinalizer(lambda: module_target_sat.cli.Subnet.delete({'id': subnet['id']}))
+    options['id'] = subnet['id']
     with pytest.raises(CLIReturnCodeError, match='Could not update the subnet:'):
         module_target_sat.cli.Subnet.update(options)
     # check - subnet is not updated
@@ -223,8 +223,8 @@ def test_negative_update_address_pool(request, options, module_target_sat):
     :CaseImportance: Medium
     """
     subnet = module_target_sat.cli_factory.make_subnet()
-    opts = {'id': subnet['id']}
     request.addfinalizer(lambda: module_target_sat.cli.Subnet.delete({'id': subnet['id']}))
+    opts = {'id': subnet['id']}
     # generate pool range from network address
     for key, val in options.items():
         opts[key] = re.sub(r'\d+$', str(val), subnet['network-addr'])

tests/foreman/destructive/test_capsule_loadbalancer.py

@@ -191,6 +191,7 @@ def test_loadbalancer_install_package(
         registration.
 
         """
+
     # Register content host
     result = rhel7_contenthost.register(
         org=module_org,
@@ -219,6 +220,9 @@ def test_loadbalancer_install_package(
         if loadbalancer_setup['setup_capsules']['capsule_1'].hostname in result.stdout
         else loadbalancer_setup['setup_capsules']['capsule_2']
     )
+    request.addfinalizer(
+        lambda: registered_to_capsule.power_control(state=VmState.RUNNING, ensure=True)
+    )
 
     # Remove the packages from the client
     result = rhel7_contenthost.execute('yum remove -y tree')
@@ -231,10 +235,6 @@ def test_loadbalancer_install_package(
     result = rhel7_contenthost.execute('yum install -y tree')
     assert result.status == 0
 
-    @request.addfinalizer
-    def _finalize():
-        registered_to_capsule.power_control(state=VmState.RUNNING, ensure=True)
-
 
 @pytest.mark.rhel_ver_match('[^6]')
 @pytest.mark.tier1