From 8f47fcedf6452b1a99944db9b2f7a7dcc16846ca Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 27 Mar 2024 17:32:41 +0800 Subject: [PATCH 1/2] test/cql-pytest: extract scylla_error() for not allowed options test currently, our homebrew formatter formats `std::map` like {{k1, v1}, {k2, v2}} while {fmt} formats a map like: {k1: v1, k2: v2} and if the type of key/value is string, {fmt} quotes it, so a compaction strategy option is formatted like {"max_threshold": "1"} as we are switching to the formatters provided by {fmt}, would be better to support its convention directly. so, in this change, to prepare the change, before migrating to {fmt}, let's refactor the test to support both formats by extracting a helper to format the error message, so that we can change it to emit both formats. Refs #13245 Signed-off-by: Kefu Chai --- .../cql-pytest/test_compaction_strategy_validation.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/cql-pytest/test_compaction_strategy_validation.py b/test/cql-pytest/test_compaction_strategy_validation.py index 3a80748bb9..4a01689e82 100644 --- a/test/cql-pytest/test_compaction_strategy_validation.py +++ b/test/cql-pytest/test_compaction_strategy_validation.py @@ -50,6 +50,11 @@ def test_leveled_compaction_strategy_options(cql, table1): assert_throws(cql, table1, r"sstable_size_in_mb value \(-5\) must be positive|sstable_size_in_mb must be larger than 0, but was -5", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'sstable_size_in_mb' : -5 }") def test_not_allowed_options(cql, table1): - assert_throws(cql, table1, r"Invalid compaction strategy options {{abc, -54.54}} for chosen strategy type|Properties specified \[abc\] are not understood by SizeTieredCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }") - assert_throws(cql, table1, r"Invalid compaction strategy options {{dog, 3}} for chosen strategy type|Properties specified \[dog\] are not understood by TimeWindowCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }") - assert_throws(cql, table1, r"Invalid compaction strategy options {{compaction_window_size, 4}} for chosen strategy type|Properties specified \[compaction_window_size\] are not understood by LeveledCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'compaction_window_size' : 4 }") + def scylla_error(**kwargs): + template = "Invalid compaction strategy options {{{}}} for chosen strategy type" + options = ', '.join(f"{{{k}, {v}}}" for k, v in kwargs.items()) + return template.format(options) + + assert_throws(cql, table1, rf"{scylla_error(abc=-54.54)}|Properties specified \[abc\] are not understood by SizeTieredCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }") + assert_throws(cql, table1, rf"{scylla_error(dog=3)}||Properties specified \[dog\] are not understood by TimeWindowCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }") + assert_throws(cql, table1, rf"{scylla_error(compaction_window_size=4)}|Properties specified \[compaction_window_size\] are not understood by LeveledCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'compaction_window_size' : 4 }") From 1632fbbef9c9e4d7b02a8a892347c9c24def38c4 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 27 Mar 2024 17:46:24 +0800 Subject: [PATCH 2/2] test/cql-pytest: match error message formated using {fmt} currently, our homebrew formatter formats `std::map` like {{k1, v1}, {k2, v2}} while {fmt} formats a map like: {k1: v1, k2: v2} and if the type of key/value is string, {fmt} quotes it, so a compaction strategy option is formatted like {"max_threshold": "1"} before switching the formatter to the ones supported by {fmt}, let's update the test to match with the new format. this should reduce the overhead of reviewing the change of switching the formatter. we can revert this change, and use a simpler approach after the change of formatter lands. Refs #13245 Signed-off-by: Kefu Chai --- test/cql-pytest/test_compaction_strategy_validation.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/cql-pytest/test_compaction_strategy_validation.py b/test/cql-pytest/test_compaction_strategy_validation.py index 4a01689e82..4e4025ae5c 100644 --- a/test/cql-pytest/test_compaction_strategy_validation.py +++ b/test/cql-pytest/test_compaction_strategy_validation.py @@ -52,8 +52,13 @@ def test_leveled_compaction_strategy_options(cql, table1): def test_not_allowed_options(cql, table1): def scylla_error(**kwargs): template = "Invalid compaction strategy options {{{}}} for chosen strategy type" - options = ', '.join(f"{{{k}, {v}}}" for k, v in kwargs.items()) - return template.format(options) + # TODO: remove the old old_options + # {fmt} formats map like {k1: v1, k2: v2}, while existing operator<< + # formatter formats like {{k1, v1}, {k2, v2}}, so cater both formats + # before switching to {fmt}'s formatter. + old_options = ', '.join(f"{{{k}, {v}}}" for k, v in kwargs.items()) + options = ', '.join(f"\"{k}\": \"{v}\"" for k, v in kwargs.items()) + return '|'.join([template.format(old_options), template.format(options)]) assert_throws(cql, table1, rf"{scylla_error(abc=-54.54)}|Properties specified \[abc\] are not understood by SizeTieredCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }") assert_throws(cql, table1, rf"{scylla_error(dog=3)}||Properties specified \[dog\] are not understood by TimeWindowCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }")