You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/01/22 08:32:53 UTC
[GitHub] [cassandra-dtest] tlasica commented on a change in pull request #115: CASSANDRA-16399: Fix collecting resource intensive tests
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org