scylladb / python-driver

ScyllaDB Python Driver, originally DataStax Python Driver for Apache Cassandra
https://python-driver.docs.scylladb.com
Apache License 2.0
70 stars 42 forks source link

Resurrect integration tests #207

Open Lorak-mmk opened 1 year ago

Lorak-mmk commented 1 year ago

Our integration tests are in a pretty sorry state, which isn't good considering we use this driver for pretty much all Scylla tests. I'd like to improve this situation so I'm opening this issue to keep track of those efforts and the current state. I'm describing the problems as I understand them, and I'm not an expert in this codebase. If anything here is incorrect feel free to correct me.

Problem 1: We run only a small fraction of integration tests in CI:

./ci/run_integration_test.sh tests/integration/standard/test_authentication.py tests/integration/standard/test_cluster.py tests/integration/standard/test_concurrent.py tests/integration/standard/test_connection.py tests/integration/standard/test_control_connection.py tests/integration/standard/test_custom_payload.py tests/integration/standard/test_custom_protocol_handler.py tests/integration/standard/test_cython_protocol_handlers.py tests/integration/standard/test_scylla_cloud.py tests/integration/standard/test_use_keyspace.py tests/integration/standard/test_ip_change.py

I think we should:

Problem 2: We only test with Scylla, and with 1 version of it.

This is pretty self explanatory. I think we should run integration tests with:

Problem 3: Tests are slow

Tests are run sequentially, and each one needs to setup a cluster - unless the previous one was using cluster with the same topology. This is quite slow, considering very long Scylla startup times. Afaik this is the limitation of tests/integration/__init__.py (unless there is some limitation I don't know about, like ccm not being able to run >1 cluster), that has a global var to hold a cluster - so it can only use one at a time.

tests/integration/__init__.py in general seems quite messy to me - maybe we could refactor it, and remove this limitation in the process?

Problem 4: We only run those tests with asyncio reactor

That one I'm not sure is important or even correct. I can see that there are more reactors supported by driver (and we do run some unit tests on some of those), yet we only use asyncio libev during integration tests. Shouldn't we run some tests with all the other ones?

Problem 5: A lot of unnecessary logs from ccm (I guess?).

Setup / teardown logs are massive - they cause my terminal to lag, and require some time to scroll trough. Most of the time they are also completely useless. They should be hidden by default and only displayed when some env variable is set.

Problem 6: Multiple warnings

There are a lot of warnings printed when executing integration test. Not all of them are our fault, but we should fix those that we can, as they make it harder to read the results.

Example log from cqlengine tests ``` venv/lib64/python3.11/site-packages/eventlet/support/greenlets.py:6 /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/eventlet/support/greenlets.py:6: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead. preserves_excinfo = (distutils.version.LooseVersion(greenlet.__version__) venv/lib64/python3.11/site-packages/eventlet/support/greenlets.py:7 /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/eventlet/support/greenlets.py:7: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead. >= distutils.version.LooseVersion('0.3.2')) venv/lib64/python3.11/site-packages/eventlet/green/OpenSSL/crypto.py:1 /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/eventlet/green/OpenSSL/crypto.py:1: DeprecationWarning: PKCS#7 support in pyOpenSSL is deprecated. You should use the APIs in cryptography. from OpenSSL.crypto import * venv/lib64/python3.11/site-packages/eventlet/green/OpenSSL/crypto.py:1 /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/eventlet/green/OpenSSL/crypto.py:1: DeprecationWarning: PKCS#12 support in pyOpenSSL is deprecated. You should use the APIs in cryptography. from OpenSSL.crypto import * tests/integration/cqlengine/test_batch_query.py:26 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_batch_query.py:26: PytestCollectionWarning: cannot collect test class 'TestMultiKeyModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_batch_query.py) class TestMultiKeyModel(Model): tests/integration/__init__.py:1025 /home/karolbaryla/repos/python-driver/tests/integration/__init__.py:1025: PytestCollectionWarning: cannot collect test class 'TestCluster' because it has a __new__ constructor (from: tests/integration/cqlengine/test_connections.py) class TestCluster(object): tests/integration/cqlengine/test_connections.py:28 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_connections.py:28: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_connections.py) class TestModel(Model): tests/integration/cqlengine/test_consistency.py:28 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_consistency.py:28: PytestCollectionWarning: cannot collect test class 'TestConsistencyModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_consistency.py) class TestConsistencyModel(Model): tests/integration/cqlengine/test_context_query.py:22 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_context_query.py:22: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_context_query.py) class TestModel(Model): tests/integration/cqlengine/test_ifexists.py:28 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ifexists.py:28: PytestCollectionWarning: cannot collect test class 'TestIfExistsModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ifexists.py) class TestIfExistsModel(Model): tests/integration/cqlengine/test_ifexists.py:35 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ifexists.py:35: PytestCollectionWarning: cannot collect test class 'TestIfExistsModel2' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ifexists.py) class TestIfExistsModel2(Model): tests/integration/cqlengine/test_ifexists.py:42 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ifexists.py:42: PytestCollectionWarning: cannot collect test class 'TestIfExistsWithCounterModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ifexists.py) class TestIfExistsWithCounterModel(Model): tests/integration/cqlengine/test_ifnotexists.py:27 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ifnotexists.py:27: PytestCollectionWarning: cannot collect test class 'TestIfNotExistsModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ifnotexists.py) class TestIfNotExistsModel(Model): tests/integration/cqlengine/test_ifnotexists.py:34 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ifnotexists.py:34: PytestCollectionWarning: cannot collect test class 'TestIfNotExistsWithCounterModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ifnotexists.py) class TestIfNotExistsWithCounterModel(Model): tests/integration/cqlengine/test_lwt_conditional.py:30 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_lwt_conditional.py:30: PytestCollectionWarning: cannot collect test class 'TestConditionalModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_lwt_conditional.py) class TestConditionalModel(Model): tests/integration/cqlengine/test_lwt_conditional.py:36 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_lwt_conditional.py:36: PytestCollectionWarning: cannot collect test class 'TestUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_lwt_conditional.py) class TestUpdateModel(Model): tests/integration/cqlengine/test_timestamp.py:27 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_timestamp.py:27: PytestCollectionWarning: cannot collect test class 'TestTimestampModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_timestamp.py) class TestTimestampModel(Model): tests/integration/cqlengine/test_ttl.py:31 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ttl.py:31: PytestCollectionWarning: cannot collect test class 'TestTTLModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ttl.py) class TestTTLModel(Model): tests/integration/cqlengine/test_ttl.py:50 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_ttl.py:50: PytestCollectionWarning: cannot collect test class 'TestDefaultTTLModel' because it has a __init__ constructor (from: tests/integration/cqlengine/test_ttl.py) class TestDefaultTTLModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:38 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:38: PytestCollectionWarning: cannot collect test class 'TestSetModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestSetModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:196 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:196: PytestCollectionWarning: cannot collect test class 'TestListModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestListModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:350 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:350: PytestCollectionWarning: cannot collect test class 'TestMapModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestMapModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:528 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:528: PytestCollectionWarning: cannot collect test class 'TestCamelMapModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestCamelMapModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:549 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:549: PytestCollectionWarning: cannot collect test class 'TestTupleModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestTupleModel(Model): tests/integration/cqlengine/columns/test_container_columns.py:749 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_container_columns.py:749: PytestCollectionWarning: cannot collect test class 'TestNestedModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_container_columns.py) class TestNestedModel(Model): tests/integration/cqlengine/columns/test_counter_column.py:23 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_counter_column.py:23: PytestCollectionWarning: cannot collect test class 'TestCounterModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_counter_column.py) class TestCounterModel(Model): tests/integration/cqlengine/columns/test_static_column.py:30 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/columns/test_static_column.py:30: PytestCollectionWarning: cannot collect test class 'TestStaticModel' because it has a __init__ constructor (from: tests/integration/cqlengine/columns/test_static_column.py) class TestStaticModel(Model): tests/integration/__init__.py:1025 /home/karolbaryla/repos/python-driver/tests/integration/__init__.py:1025: PytestCollectionWarning: cannot collect test class 'TestCluster' because it has a __new__ constructor (from: tests/integration/cqlengine/connections/test_connection.py) class TestCluster(object): tests/integration/cqlengine/connections/test_connection.py:31 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/connections/test_connection.py:31: PytestCollectionWarning: cannot collect test class 'TestConnectModel' because it has a __init__ constructor (from: tests/integration/cqlengine/connections/test_connection.py) class TestConnectModel(Model): tests/integration/cqlengine/query/test_queryset.py:64 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/query/test_queryset.py:64: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/management/test_management.py) class TestModel(Model): tests/integration/cqlengine/management/test_management.py:374 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/management/test_management.py:374: PytestCollectionWarning: cannot collect test class 'TestIndexSetModel' because it has a __init__ constructor (from: tests/integration/cqlengine/management/test_management.py) class TestIndexSetModel(Model): tests/integration/cqlengine/model/test_equality_operations.py:23 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_equality_operations.py:23: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_equality_operations.py) class TestModel(Model): tests/integration/cqlengine/base.py:24 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/base.py:24: PytestCollectionWarning: cannot collect test class 'TestQueryUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_model.py) class TestQueryUpdateModel(Model): tests/integration/cqlengine/model/test_model_io.py:38 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_model_io.py:38: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_model_io.py) class TestModel(Model): tests/integration/cqlengine/model/test_model_io.py:46 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_model_io.py:46: PytestCollectionWarning: cannot collect test class 'TestModelSave' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_model_io.py) class TestModelSave(Model): tests/integration/cqlengine/model/test_model_io.py:304 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_model_io.py:304: PytestCollectionWarning: cannot collect test class 'TestMultiKeyModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_model_io.py) class TestMultiKeyModel(Model): tests/integration/cqlengine/model/test_model_io.py:660 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_model_io.py:660: PytestCollectionWarning: cannot collect test class 'TestQueryModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_model_io.py) class TestQueryModel(Model): tests/integration/cqlengine/model/test_updates.py:27 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_updates.py:27: PytestCollectionWarning: cannot collect test class 'TestUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_updates.py) class TestUpdateModel(Model): tests/integration/cqlengine/model/test_value_lists.py:24 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_value_lists.py:24: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_value_lists.py) class TestModel(Model): tests/integration/cqlengine/model/test_value_lists.py:29 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_value_lists.py:29: PytestCollectionWarning: cannot collect test class 'TestClusteringComplexModel' because it has a __init__ constructor (from: tests/integration/cqlengine/model/test_value_lists.py) class TestClusteringComplexModel(Model): tests/integration/cqlengine/base.py:24 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/base.py:24: PytestCollectionWarning: cannot collect test class 'TestQueryUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/operators/test_where_operators.py) class TestQueryUpdateModel(Model): tests/integration/cqlengine/query/test_batch_query.py:29 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/query/test_batch_query.py:29: PytestCollectionWarning: cannot collect test class 'TestMultiKeyModel' because it has a __init__ constructor (from: tests/integration/cqlengine/query/test_batch_query.py) class TestMultiKeyModel(Model): tests/integration/__init__.py:1025 /home/karolbaryla/repos/python-driver/tests/integration/__init__.py:1025: PytestCollectionWarning: cannot collect test class 'TestCluster' because it has a __new__ constructor (from: tests/integration/cqlengine/query/test_queryset.py) class TestCluster(object): tests/integration/cqlengine/query/test_queryset.py:64 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/query/test_queryset.py:64: PytestCollectionWarning: cannot collect test class 'TestModel' because it has a __init__ constructor (from: tests/integration/cqlengine/query/test_queryset.py) class TestModel(Model): tests/integration/cqlengine/query/test_queryset.py:106 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/query/test_queryset.py:106: PytestCollectionWarning: cannot collect test class 'TestMultiClusteringModel' because it has a __init__ constructor (from: tests/integration/cqlengine/query/test_queryset.py) class TestMultiClusteringModel(Model): tests/integration/cqlengine/query/test_queryset.py:1348 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/query/test_queryset.py:1348: PytestCollectionWarning: cannot collect test class 'TestModelSmall' because it has a __init__ constructor (from: tests/integration/cqlengine/query/test_queryset.py) class TestModelSmall(Model): tests/integration/cqlengine/base.py:24 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/base.py:24: PytestCollectionWarning: cannot collect test class 'TestQueryUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/query/test_updates.py) class TestQueryUpdateModel(Model): tests/integration/cqlengine/base.py:24 /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/base.py:24: PytestCollectionWarning: cannot collect test class 'TestQueryUpdateModel' because it has a __init__ constructor (from: tests/integration/cqlengine/statements/test_base_statement.py) class TestQueryUpdateModel(Model): tests/integration/__init__.py:1025 /home/karolbaryla/repos/python-driver/tests/integration/__init__.py:1025: PytestCollectionWarning: cannot collect test class 'TestCluster' because it has a __new__ constructor (from: tests/integration/cqlengine/statements/test_base_statement.py) class TestCluster(object): tests/integration/cqlengine/test_batch_query.py::BatchQueryTests::test_bulk_delete_success_case /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/ccmlib/scylla_repository.py:75: ResourceWarning: unclosed files_in_bucket = aws_bucket_ls(s3_url) Enable tracemalloc to get traceback where the object was allocated. See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info. tests/integration/cqlengine/test_batch_query.py::BatchQueryTests::test_bulk_delete_success_case /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/ccmlib/scylla_repository.py:75: ResourceWarning: unclosed files_in_bucket = aws_bucket_ls(s3_url) Enable tracemalloc to get traceback where the object was allocated. See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info. tests/integration/cqlengine/test_batch_query.py: 1 warning tests/integration/cqlengine/test_connections.py: 5 warnings tests/integration/cqlengine/columns/test_container_columns.py: 1 warning tests/integration/cqlengine/connections/test_connection.py: 4 warnings /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cluster.py:2475: DeprecationWarning: Setting the consistency level at the session level will be removed in 4.0. Consider using execution profiles and setting the desired consistency level to the EXEC_PROFILE_DEFAULT profile. warn("Setting the consistency level at the session level will be removed in 4.0. Consider using " tests/integration/cqlengine/test_batch_query.py::BatchQueryCallbacksTests::test_callbacks_work_multiple_times /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/test_batch_query.py:232: DeprecationWarning: Please use assertRegex instead. self.assertRegexpMatches(str(w[0].message), r"^Batch.*multiple.*") tests/integration/cqlengine/test_ttl.py::TTLDefaultTest::test_default_ttl_modify tests/integration/cqlengine/test_ttl.py::TTLDefaultTest::test_default_ttl_modify tests/integration/cqlengine/test_ttl.py::TTLDefaultTest::test_default_ttl_not_set tests/integration/cqlengine/test_ttl.py::TTLDefaultTest::test_default_ttl_set tests/integration/cqlengine/columns/test_container_columns.py::TestListColumn::test_partial_updates tests/integration/cqlengine/statements/test_base_statement.py::ExecuteStatementTest::test_insert_statement_execute tests/integration/cqlengine/statements/test_base_statement.py::ExecuteStatementTest::test_insert_statement_execute /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cluster.py:5323: DeprecationWarning: ResultSet indexing support will be removed in 4.0. Consider using ResultSet.one() to get a single row. warn("ResultSet indexing support will be removed in 4.0. Consider using " tests/integration/cqlengine/columns/test_counter_column.py: 8 warnings tests/integration/cqlengine/query/test_batch_query.py: 6 warnings /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cqlengine/query.py:1483: DeprecationWarning: 'create' and 'save' actions on Counters are deprecated. It will be disallowed in 4.0. Use the 'update' mechanism instead. warn("'create' and 'save' actions on Counters are deprecated. It will be disallowed in 4.0. " tests/integration/cqlengine/connections/test_connection.py::ConnectionInitTest::test_connection_from_session_with_legacy_settings /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cluster.py:1308: DeprecationWarning: Legacy execution parameters will be removed in 4.0. Consider using execution profiles. warn("Legacy execution parameters will be removed in 4.0. Consider using " tests/integration/cqlengine/management/test_compaction_settings.py::AlterTableTest::test_alter_actually_alters /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/management/test_compaction_settings.py:86: DeprecationWarning: Please use assertRegex instead. self.assertRegexpMatches(table_meta.export_as_string(), '.*SizeTieredCompactionStrategy.*') tests/integration/cqlengine/management/test_compaction_settings.py::AlterTableTest::test_alter_options /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/management/test_compaction_settings.py:100: DeprecationWarning: Please use assertRegex instead. self.assertRegexpMatches(table_meta.export_as_string(), ".*'sstable_size_in_mb': '64'.*") tests/integration/cqlengine/management/test_compaction_settings.py::AlterTableTest::test_alter_options /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/management/test_compaction_settings.py:104: DeprecationWarning: Please use assertRegex instead. self.assertRegexpMatches(table_meta.export_as_string(), ".*'sstable_size_in_mb': '128'.*") tests/integration/cqlengine/management/test_management.py::TablePropertiesTests::test_bogus_option_update /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/management/test_management.py:264: DeprecationWarning: Please use assertRaisesRegex instead. self.assertRaisesRegexp(KeyError, "Invalid table option.*%s.*" % option, sync_table, ModelWithTableProperties) tests/integration/cqlengine/management/test_management.py::TablePropertiesTests::test_table_property_update /usr/lib64/python3.11/unittest/case.py:1176: DeprecationWarning: assertDictContainsSubset is deprecated warnings.warn('assertDictContainsSubset is deprecated', tests/integration/cqlengine/management/test_management.py::InconsistentTable::test_sync_warnings tests/integration/cqlengine/management/test_management.py::IndexTests::test_sync_index tests/integration/cqlengine/management/test_management.py::IndexTests::test_sync_index_case_sensitive tests/integration/cqlengine/management/test_management.py::IndexTests::test_sync_index_case_sensitive tests/integration/cqlengine/management/test_management.py::IndexTests::test_sync_index_case_sensitive tests/integration/cqlengine/model/test_class_construction.py::TestManualTableNamingCaseSensitive::test_proper_table_naming_case_sensitive tests/integration/cqlengine/model/test_model.py::TestModel::test_column_family_case_sensitive /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cqlengine/models.py:570: PendingDeprecationWarning: Model __table_name_case_sensitive__ will be removed in 4.0. warn("Model __table_name_case_sensitive__ will be removed in 4.0.", PendingDeprecationWarning) tests/integration/cqlengine/management/test_management.py::InconsistentTable::test_sync_warnings /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cqlengine/management.py:255: UserWarning: [Connection: DEFAULT_CONNECTION, Keyspace: cqlengine_test] Existing table cqlengine_test.inconsistent has column "second_key" with a type (int) differing from the model type (text). Model should be updated. warnings.warn(msg) tests/integration/cqlengine/management/test_management.py::InconsistentTable::test_sync_warnings /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cqlengine/management.py:366: UserWarning: [Connection: DEFAULT_CONNECTION, Keyspace: cqlengine_test] Existing user type cqlengine_test.type_inconsistent has field "name" with a type (text) differing from the model user type (int). UserType should be updated. warnings.warn(msg) tests/integration/cqlengine/model/test_class_construction.py::TestModelClassFunction::test_attempting_to_make_duplicate_column_names_fails /home/karolbaryla/repos/python-driver/tests/integration/cqlengine/model/test_class_construction.py:94: DeprecationWarning: Please use assertRaisesRegex instead. with self.assertRaisesRegexp(ModelException, r".*more than once$"): tests/integration/cqlengine/model/test_class_construction.py::TestManualTableNamingCaseSensitive::test_proper_table_naming_case_insensitive /home/karolbaryla/repos/python-driver/venv/lib64/python3.11/site-packages/cassandra/cqlengine/models.py:575: UserWarning: Model __table_name__ will be case sensitive by default in 4.0. You should fix the __table_name__ value of the 'RenamedCaseInsensitiveTest' model. warn(("Model __table_name__ will be case sensitive by default in 4.0. " -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ```
Lorak-mmk commented 1 year ago

Added problems 5 and 6

Lorak-mmk commented 1 year ago

First PR regarding those problems: https://github.com/scylladb/python-driver/pull/210

fruch commented 1 year ago

On note on the removal of some code and handling of warning, keep in mind it might complex a bit the syncing with upstream (even that upstream doesn't move at a very fast pace)

regarding testing with multiple reactors/backends, unfortunately they are not exactly equivalent, and you can run all the test ontop of all of them. also the main one that is most battle proven from my POV is the libev one, it's the default once it's compiled, and that the one we use in dtest, when using any other, the behavior in some situation isn't the same, especially on scylla node restarts.

I don't even remember why asyncio is used in integration tests, but recently I switched it to libev a44418874850c74f1fc7b359a3de1a20b26d7385, exactly cause of sni_proxy tests that where using multiple ConncectionClasses

fruch commented 1 year ago

@Lorak-mmk @avelanarius, I've added you both to: https://github.com/orgs/scylladb/teams/python-driver-maint

nyh commented 1 year ago
* Remove tests that use DSE-specific functionality. I don't think we support such functionality, so it would we better to yank all DSE-specific code from the driver - to reduce complexity and avoid confusion (e.g. DSE Cloud vs Scylla Cloud). This includes removing `tests/integration/cloud` and most of `tests/integration/advanced`, along with associated code in the driver and documentation.

If you plan to remove the DSE-specific functionality itself, then it makes sense to remove the tests. But if you leave the functionality, it would be a bad idea to remove the tests, as this can lead to these DSE-specific features in our driver to break without anyone noticing.

Do we plan to remove the DSE-specific functionality? This can hurt our driver's ability to become the "default" Cassandra driver, but do we have such a goal?

* I'm not sure what to do about `tests/integration/simulacron` and what even are those - more research needed. If they can be used with Scylla - great. If not, let's not delete them - we can use them with Cassandra only.

https://github.com/datastax/simulacron

It's meant to simulate various protocol corner cases without a real server (without Cassandra or Scylla).

Problem 2: We only test with Scylla, and with 1 version of it.

This is pretty self explanatory. I think we should run integration tests with:

* Currently supported stable Scylla releases (right now: 5.1 and 5.0)

* Latest Cassandra stable release

Right. Very important in my opinion.

* Optional: upcoming Scylla stable release.

My personal favorite approach: CI should run against a specific version of Scylla (e.g., 5.1), but when a developer runs the test, it runs against whatever Scylla version this developer might have lying around - or docker if there is none.

By the way, check out Scylla's test/cql-pytest/run and test/cql-pytest/run-cassandra for example scripts that run Scylla or Cassandra very quickly. Or, I guess, you can also use ccm...

Problem 3: Tests are slow

Tests are run sequentially, and each one needs to setup a cluster - unless the previous one was using cluster with the same topology. This is quite slow, considering very long Scylla startup times. Afaik this is the limitation of tests/integration/__init__.py (unless there is some limitation I don't know about, like ccm not being able to run >1 cluster), that has a global var to hold a cluster - so it can only use one at a time.

This is very important, and the main reason why developers like me avoid dtest like the plague ;-) You can use ideas from Scylla's test/cql-pytest for single-node tests or test/topology for multi-node tests, or come up with a simpler test framework. In particular "ccm" was a good idea, but an inconvenient and much-too-slow implementation.

fruch commented 1 year ago
* Remove tests that use DSE-specific functionality. I don't think we support such functionality, so it would we better to yank all DSE-specific code from the driver - to reduce complexity and avoid confusion (e.g. DSE Cloud vs Scylla Cloud). This includes removing `tests/integration/cloud` and most of `tests/integration/advanced`, along with associated code in the driver and documentation.

If you plan to remove the DSE-specific functionality itself, then it makes sense to remove the tests. But if you leave the functionality, it would be a bad idea to remove the tests, as this can lead to these DSE-specific features in our driver to break without anyone noticing.

Do we plan to remove the DSE-specific functionality? This can hurt our driver's ability to become the "default" Cassandra driver, but do we have such a goal?

Why would any of our drivers have this as a goal ?

From my POV removing that code, adds the the complexity to taking code from upstream, and if we are o.k. with making that process more painful, then yes yank all of that code.

* I'm not sure what to do about `tests/integration/simulacron` and what even are those - more research needed. If they can be used with Scylla - great. If not, let's not delete them - we can use them with Cassandra only.

https://github.com/datastax/simulacron

It's meant to simulate various protocol corner cases without a real server (without Cassandra or Scylla).

Problem 2: We only test with Scylla, and with 1 version of it.

This is pretty self explanatory. I think we should run integration tests with:

* Currently supported stable Scylla releases (right now: 5.1 and 5.0)

* Latest Cassandra stable release

Right. Very important in my opinion.

* Optional: upcoming Scylla stable release.

My personal favorite approach: CI should run against a specific version of Scylla (e.g., 5.1), but when a developer runs the test, it runs against whatever Scylla version this developer might have lying around - or docker if there is none.

By the way, check out Scylla's test/cql-pytest/run and test/cql-pytest/run-cassandra for example scripts that run Scylla or Cassandra very quickly. Or, I guess, you can also use ccm...

Tests are already using ccm

Problem 3: Tests are slow

Tests are run sequentially, and each one needs to setup a cluster - unless the previous one was using cluster with the same topology. This is quite slow, considering very long Scylla startup times. Afaik this is the limitation of tests/integration/__init__.py (unless there is some limitation I don't know about, like ccm not being able to run >1 cluster), that has a global var to hold a cluster - so it can only use one at a time.

This is very important, and the main reason why developers like me avoid dtest like the plague ;-) You can use ideas from Scylla's test/cql-pytest for single-node tests or test/topology for multi-node tests, or come up with a simpler test framework. In particular "ccm" was a good idea, but an inconvenient and much-too-slow implementation.

Nice to see you guys running into re implementation of almost the same.

Anyhow tests here are using ccm, and the problems they have are not related to ccm, ccm can easily run multiple clusters at the same time, and doesn't have any globals as described here. and can be as fast as scylla can boot.

mykaul commented 1 year ago

Any updates on this epic?

Lorak-mmk commented 1 year ago

There is an open PR: https://github.com/scylladb/python-driver/pull/219 - I need to fix a flaky test there before it can merged. I was on a leave last week, that's why I didn't do it. After it's merged, most important things from section 1 will be fixed.

After that, I plan to fix section 2 - it should be quick and easy to do. The rest require more work, not sure when I'll get around to them.