tlasica commented on a change in pull request #115:
URL: https://github.com/apache/cassandra-dtest/pull/115#discussion_r562460898
##########
File path: conftest.py
##########
@@ -602,20 +609,17 @@ def pytest_collection_modifyitems(items, config):
if deselect_test:
logger.info("SKIP: Deselecting non-upgrade test %s because of
--execute-upgrade-tests-only" % item.name)
- if item.get_closest_marker("resource_intensive") and not collect_only:
- force_resource_intensive =
config.getoption("--force-resource-intensive-tests")
- skip_resource_intensive =
config.getoption("--skip-resource-intensive-tests")
- if not force_resource_intensive:
- if skip_resource_intensive:
- deselect_test = True
- logger.info("SKIP: Deselecting test %s as test marked
resource_intensive. To force execution of "
- "this test re-run with the
--force-resource-intensive-tests command line argument" % item.name)
+ if item.get_closest_marker("resource_intensive"):
Review comment:
this code is so complex that IMO it would be reasonable to add tests in
`meta_tests` with acceptance criteria validation, so that one does not have to
reverse engineer the rules.
decision making for single `item` should be extracted to either a class or a
function so that it can be easily tested without whole configuration burden.
##########
File path: conftest.py
##########
@@ -594,6 +594,13 @@ def pytest_collection_modifyitems(items, config):
sufficient_system_resources_resource_intensive =
sufficient_system_resources_for_resource_intensive_tests()
logger.debug("has sufficient resources? %s" %
sufficient_system_resources_resource_intensive)
+ force_resource_intensive =
config.getoption("--force-resource-intensive-tests")
+ skip_resource_intensive =
config.getoption("--skip-resource-intensive-tests")
+ only_resource_intensive =
config.getoption("--only-resource-intensive-tests")
+ if skip_resource_intensive and only_resource_intensive:
Review comment:
also `skip` and `force` should not be combined?
##########
File path: conftest.py
##########
@@ -602,20 +609,17 @@ def pytest_collection_modifyitems(items, config):
if deselect_test:
logger.info("SKIP: Deselecting non-upgrade test %s because of
--execute-upgrade-tests-only" % item.name)
- if item.get_closest_marker("resource_intensive") and not collect_only:
- force_resource_intensive =
config.getoption("--force-resource-intensive-tests")
- skip_resource_intensive =
config.getoption("--skip-resource-intensive-tests")
- if not force_resource_intensive:
- if skip_resource_intensive:
- deselect_test = True
- logger.info("SKIP: Deselecting test %s as test marked
resource_intensive. To force execution of "
- "this test re-run with the
--force-resource-intensive-tests command line argument" % item.name)
+ if item.get_closest_marker("resource_intensive"):
+ if skip_resource_intensive:
+ deselect_test = True
+ logger.info("SKIP: Deselecting test %s as test marked
resource_intensive. To force execution of "
+ "this test re-run with the
--force-resource-intensive-tests command line argument" % item.name)
+ elif not force_resource_intensive:
if not sufficient_system_resources_resource_intensive:
deselect_test = True
logger.info("SKIP: Deselecting resource_intensive test %s
due to insufficient system resources" % item.name)
- if not item.get_closest_marker("resource_intensive") and not
collect_only:
- only_resource_intensive =
config.getoption("--only-resource-intensive-tests")
+ if not item.get_closest_marker("resource_intensive"):
Review comment:
I wonder what was the rationale for running this logic only if `not
collect_only`?
maybe there was a valid reason not to do it always?
@michaelsembwever ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
