From de586ef8564c50a41c76bb909c0ff5c1eb7dd87a Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Tue, 8 Feb 2022 18:27:32 +0200 Subject: [PATCH] test/cql-pytest: mechanism for tests requiring raft-based schema updates Issue #8968 no longer exists when Raft-based schema updates are enabled in Scylla (with --experimental-features=raft). Before we can close this issue we need a way to re-run its test test_keyspace.py::test_concurrent_create_and_drop_keyspace with Raft and see it pass. But we also want the tests to continue to run by default the older raft-less schema updates - so that this mode doesn't regress during the potentially-long duration that it's still the default! The solution in this patch is: 1. Introduce a "--raft" option to test/cql-pytest/run, which runs the tests against a Scylla with the raft experimental feature, while the default is still to run without it. 2. Introduce a text fixture "fails_without_raft" which marks a test which is expected to fail with the old pre-raft code, but is expected to pass in the new code. 3. Mark the test test_concurrent_create_and_drop_keyspace with this new "fails_without_raft". After this patch, running test/cql-pytest/run --raft test_keyspace.py::test_concurrent_create_and_drop_keyspace Passes, which shows that issue 8968 was fixed (in Raft mode) - so we can say: Fixes #8968 Running the same test without "--raft" still xfails (an expected failure). Signed-off-by: Nadav Har'El Message-Id: <20220208162732.260888-1-nyh@scylladb.com> --- test/cql-pytest/conftest.py | 22 ++++++++++++++++++++++ test/cql-pytest/run | 12 ++++++++++++ test/cql-pytest/test_keyspace.py | 3 +-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/test/cql-pytest/conftest.py b/test/cql-pytest/conftest.py index 3e60573019..f187540848 100644 --- a/test/cql-pytest/conftest.py +++ b/test/cql-pytest/conftest.py @@ -122,5 +122,27 @@ def cassandra_bug(cql): if not any('scylla' in name for name in names): pytest.xfail('A known Cassandra bug') +# While the raft-based schema modifications are still experimental and only +# optionally enabled (the "--raft" option to test/cql-pytest/run enables it +# in Scylla), some tests are expected to fail on Scylla without this option +# enabled, and pass with it enabled (and also pass on Cassandra). These tests +# should use the "fails_without_raft" fixture. When Raft mode becomes the +# default, this fixture can be removed. +@pytest.fixture(scope="session") +def check_pre_raft(cql): + # If not running on Scylla, return false. + names = [row.table_name for row in cql.execute("SELECT * FROM system_schema.tables WHERE keyspace_name = 'system'")] + if not any('scylla' in name for name in names): + return false + # In Scylla, we check Raft mode by inspecting the configuration via CQL. + experimental_features = list(cql.execute("SELECT value FROM system.config WHERE name = 'experimental_features'"))[0].value + # Because of https://github.com/scylladb/scylla/issues/10047 the features + # appear as numbers instead of strings. FIXME! + return not '\x04' in experimental_features +@pytest.fixture(scope="function") +def fails_without_raft(request, check_pre_raft): + if check_pre_raft: + request.node.add_marker(pytest.mark.xfail(reason='Test expected to fail without Raft experimental feature on')) + # TODO: use new_test_table and "yield from" to make shared test_table # fixtures with some common schemas. diff --git a/test/cql-pytest/run b/test/cql-pytest/run index d55fbab648..c97de07048 100755 --- a/test/cql-pytest/run +++ b/test/cql-pytest/run @@ -14,6 +14,18 @@ else: cmd = run.run_scylla_cmd check_cql = run.check_cql +# If the "--raft" option is given, switch to the experimental Raft-based +# implementation of schema operations. Some tests are expected to fail +# when not in raft mode, so they use the fails_without_raft fixture +# that will cause them to xfail when raft isn't used. +if '--raft' in sys.argv: + sys.argv.remove('--raft') + def run_with_raft(pid, dir): + (c, e) = run_with_raft.orig_cmd(pid, dir) + return (c + ['--experimental-features=raft'], e) + run_with_raft.orig_cmd = cmd + cmd = run_with_raft + pid = run.run_with_temporary_dir(cmd) ip = run.pid_to_ip(pid) diff --git a/test/cql-pytest/test_keyspace.py b/test/cql-pytest/test_keyspace.py index 3d916833e2..d318e0bbf1 100644 --- a/test/cql-pytest/test_keyspace.py +++ b/test/cql-pytest/test_keyspace.py @@ -142,8 +142,7 @@ def test_alter_keyspace_double_with(cql): # deleted. But we expect that at the end of the test the database remains in # some valid state - the keyspace should either exist or not exist. It # shouldn't be in some broken immortal state as reported in issue #8968. -@pytest.mark.xfail(reason="issue #8968") -def test_concurrent_create_and_drop_keyspace(cql, this_dc): +def test_concurrent_create_and_drop_keyspace(cql, this_dc, fails_without_raft): ksdef = "WITH REPLICATION = { 'class' : 'NetworkTopologyStrategy', '" + this_dc + "' : 1 }" cfdef = "(a int PRIMARY KEY)" with new_test_keyspace(cql, ksdef) as keyspace: