Use fixtures instead of finalizers #14568
Conversation
The teardown order was not correct and host was not deleted before hostgroup pytest-dev/pytest#10023 (comment)
dosas
force-pushed
the
fix-finalizer-execution-order
branch
from
8854234 to
e931f8d
Compare
|
@dosas , I have unresolved comments, please go into |
Co-authored-by: vijay sawant <[email protected]>
| @pytest.fixture | ||
| def hostgroup(param_name, param_value, module_org, module_target_sat): | ||
| """Create hostgroup object, yeilds it for test cases, delete object before teardown""" | ||
| """Create hostgroup""" |
There was a problem hiding this comment.
| """Create hostgroup""" |
| @pytest.fixture | ||
| def host(param_name, param_value, module_org, module_location, module_target_sat, cp, hostgroup): | ||
| """Create host object, yeilds it for test cases, delete object before teardown""" | ||
| """Create host""" |
There was a problem hiding this comment.
| """Create host""" |
| @pytest.fixture | ||
| def cp(param_name, param_value, module_target_sat): | ||
| """Create common parameter object, yields it for test cases, delete object before teardown""" | ||
| """Create common parameter""" |
There was a problem hiding this comment.
| """Create common parameter""" |
| @pytest.fixture | ||
| def param_value(): | ||
| """Generate value string for common parameter and return to test cases""" | ||
| """Generate value for common parameter""" |
There was a problem hiding this comment.
| """Generate value for common parameter""" |
| @pytest.fixture | ||
| def param_name(): | ||
| """Generate name string for common parameter and return to test cases""" | ||
| """Generate name for common parameter""" |
There was a problem hiding this comment.
| """Generate name for common parameter""" |
| @@ -14,44 +14,84 @@ | |||
| import pytest | |||
|
|
|||
|
|
|||
| @pytest.fixture | |||
| def param_name(): | |||
There was a problem hiding this comment.
I think it would be better to just use parametrization for the test and generate the name and value there. You can pass those variables to your other fixtures using request.
There was a problem hiding this comment.
To give you an example see how repo_options works in https://github.com/SatelliteQE/robottelo/blob/master/tests/foreman/api/test_repository.py#L36
There was a problem hiding this comment.
I think this change goes a bit too far with fixture use. Keep in mind that fixtures should largely be used for setup/teardown and not test logic itself.
Also, consider breaking fixtures down into smaller components only when it make sense for them to be inherited by another. This helps to reduce the amount of fixtures being written.
|
@JacobCallahan Before we start having endless iterations, could you be a bit more specific about which fixtures you want to have removed? |
|
@dosas at a minimum, the fixtures providing the name and value could be included in another, but you can also take a larger approach. For example, to have test-specific setup/teardown in fixture form we can even do it all at once. def setup_param_precedence(module_target_sat, module_org, module_location):
param_name = gen_alpha()
param_value = gen_alpha()
cp = module_target_sat.api.CommonParameter(name=param_name, value=param_value).create()
host = module_target_sat.api.Host(organization=module_org, location=module_location).create()
hg = module_target_sat.api.HostGroup(
organization=[module_org],
group_parameters_attributes=[{'name': param_name, 'value': param_value}],
).create()
yield Box(cp=cp, host=host, hg=hg, param_name=param_name)
cp.delete()
host.delete()
hg.delete()Then in the test, you can access the values by using setup_param_precedence.cp
setup_param_precedence.hostor alias |
I chose the fixture approach to have modularity and ensure that deletion in teardown is independent. If there is no need for that I will go for the most simple solution and put the finalizers in the right order so the change will be minimal. |
|
Simpler PR #14620 |
|
|
||
| @pytest.fixture | ||
| def host(param_name, param_value, module_org, module_location, module_target_sat, cp, hostgroup): | ||
| """Create host object, yeilds it for test cases, delete object before teardown""" |
There was a problem hiding this comment.
| """Create host object, yeilds it for test cases, delete object before teardown""" | |
| """Create host object, yields it for test cases, delete object before teardown""" |
|
|
||
|
|
||
| @pytest.fixture | ||
| def host(param_name, param_value, module_org, module_location, module_target_sat, cp, hostgroup): |
There was a problem hiding this comment.
Many parameters here appear to be unused.
| def host(param_name, param_value, module_org, module_location, module_target_sat, cp, hostgroup): | |
| def host(module_org, module_location, module_target_sat): |
|
Closing in favor of #14620 |
Problem Statement
The teardown order was not correct
and host was not deleted before hostgroup
pytest-dev/pytest#10023 (comment)
Solution
Use fixtures