From 4b10f2e47bb9529dfc52fd8cbdd8e083e2a34dc2 Mon Sep 17 00:00:00 2001 From: ggivo Date: Thu, 30 Jan 2025 13:06:40 +0200 Subject: [PATCH] Test with 8.0-M04-pre (#4069) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Test with 8.0-M03-pre * Test with 8.0-M03-pre * Test with 8.0-M03 * Test with 8.0-M04-pre * SearchDefaultDialectTest.testDialectsWithFTExplain Output of ft.explain has changed in Redis 8.0. Existing Jedis client ftExplain() API returns the raw output and does not perform any parsing/mapping, hence updating the test to verify only for received output when used with correct dialect & gets syntax error otherwise. * Update testAggregationBuilderAddScores test to match change in SEARCH module replacing default SCORER from TF-IDF to BM25. * Apply suggestions from code review Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> * Address review comments * Update SearchDefaultDialectTest.java * Output of ft.explain  has changed in Redis 8.0. Existing Jedis client ftExplain() API returns the raw output  and does not perform any parsing/mapping, hence updating the test to verify only for received output when used with correct dialect & gets syntax error otherwise. * minor changes: format, private, unused variable, etc. --------- Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> --- .github/workflows/test-on-docker.yml | 2 +- Makefile | 4 +- .../jedis/modules/search/AggregationTest.java | 14 +++- .../search/SearchDefaultDialectTest.java | 82 ++++++++----------- .../jedis/modules/search/SearchTest.java | 81 ++++++------------ .../clients/jedis/util/RedisConditions.java | 77 +++++++++++++++++ 6 files changed, 149 insertions(+), 111 deletions(-) create mode 100644 src/test/java/redis/clients/jedis/util/RedisConditions.java diff --git a/.github/workflows/test-on-docker.yml b/.github/workflows/test-on-docker.yml index 5ce10e5d4f..e2a12e25be 100644 --- a/.github/workflows/test-on-docker.yml +++ b/.github/workflows/test-on-docker.yml @@ -36,7 +36,7 @@ jobs: fail-fast: false matrix: redis_version: - - "8.0-M02" + - "8.0-M04-pre" - "7.4.1" - "7.2.6" # - "6.2.16" diff --git a/Makefile b/Makefile index 275ea7e58c..116192309d 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,8 @@ PATH := ./redis-git/src:${PATH} # Supported test env versions -SUPPORTED_TEST_ENV_VERSIONS := 8.0-M02 7.4.1 7.2.6 6.2.16 -DEFAULT_TEST_ENV_VERSION := 8.0-M02 +SUPPORTED_TEST_ENV_VERSIONS := 8.0-M04-pre, 8.0-M02 7.4.1 7.2.6 6.2.16 +DEFAULT_TEST_ENV_VERSION := 8.0-M04-pre REDIS_ENV_WORK_DIR := $(or ${REDIS_ENV_WORK_DIR},/tmp/redis-env-work) define REDIS1_CONF diff --git a/src/test/java/redis/clients/jedis/modules/search/AggregationTest.java b/src/test/java/redis/clients/jedis/modules/search/AggregationTest.java index 61ad8cda61..c7b7c9ecea 100644 --- a/src/test/java/redis/clients/jedis/modules/search/AggregationTest.java +++ b/src/test/java/redis/clients/jedis/modules/search/AggregationTest.java @@ -37,6 +37,7 @@ import redis.clients.jedis.search.aggr.FtAggregateIteration; import redis.clients.jedis.search.schemafields.NumericField; import redis.clients.jedis.search.schemafields.TextField; +import redis.clients.jedis.util.RedisConditions; @RunWith(Parameterized.class) public class AggregationTest extends RedisModuleCommandsTestBase { @@ -202,7 +203,7 @@ public void testAggregationBuilderVerbatim() { } @Test - @SinceRedisVersion(value="7.4.0", message="ADDSCORES") + @SinceRedisVersion(value = "7.4.0", message = "ADDSCORES") public void testAggregationBuilderAddScores() { Schema sc = new Schema(); sc.addSortableTextField("name", 1.0); @@ -215,8 +216,15 @@ public void testAggregationBuilderAddScores() { .apply("@__score * 100", "normalized_score").dialect(3); AggregationResult res = client.ftAggregate(index, r); - assertEquals(2, res.getRow(0).getLong("__score")); - assertEquals(200, res.getRow(0).getLong("normalized_score")); + if (RedisConditions.of(client).moduleVersionIsGreatherThan("SEARCH", 79900)) { + // Default scorer is BM25 + assertEquals(0.6931, res.getRow(0).getDouble("__score"), 0.0001); + assertEquals(69.31, res.getRow(0).getDouble("normalized_score"), 0.01); + } else { + // Default scorer is TF-IDF + assertEquals(2, res.getRow(0).getLong("__score")); + assertEquals(200, res.getRow(0).getLong("normalized_score")); + } } @Test diff --git a/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java b/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java index 34adccf1d5..8192f29a1a 100644 --- a/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java +++ b/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java @@ -1,19 +1,23 @@ package redis.clients.jedis.modules.search; -import static org.junit.Assert.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyOrNullString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static redis.clients.jedis.util.AssertUtil.assertOK; import java.util.*; -import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; -import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import redis.clients.jedis.RedisProtocol; +import redis.clients.jedis.UnifiedJedis; import redis.clients.jedis.exceptions.JedisDataException; import redis.clients.jedis.search.*; import redis.clients.jedis.search.schemafields.NumericField; @@ -107,62 +111,34 @@ public void testDialectsWithFTExplain() throws Exception { String q = "(*)"; Query query = new Query(q).dialect(1); - try { - client.ftExplain(INDEX, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } - query = new Query(q); // dialect=default=2 - assertTrue("Should contain 'WILDCARD'", client.ftExplain(INDEX, query).contains("WILDCARD")); + assertSyntaxError(query, client); // dialect=1 throws syntax error + query = new Query(q); // dialect=default=2 should return execution plan + assertThat(client.ftExplain(INDEX, query), containsString("WILDCARD")); q = "$hello"; query = new Query(q).dialect(1); - try { - client.ftExplain(INDEX, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } - query = new Query(q).addParam("hello", "hello"); // dialect=default=2 - assertTrue("Should contain 'UNION {\n hello\n +hello(expanded)\n}\n'", - client.ftExplain(INDEX, query).contains("UNION {\n hello\n +hello(expanded)\n}\n")); + assertSyntaxError(query, client); // dialect=1 throws syntax error + query = new Query(q).addParam("hello", "hello"); // dialect=default=2 should return execution plan + assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString())); + q = "@title:(@num:[0 10])"; - query = new Query(q).dialect(1); - assertTrue("Should contain 'NUMERIC {0.000000 <= @num <= 10.000000}'", - client.ftExplain(INDEX, query).contains("NUMERIC {0.000000 <= @num <= 10.000000}")); + query = new Query(q).dialect(1); // dialect=1 should return execution plan + assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString())); query = new Query(q); // dialect=default=2 - try { - client.ftExplain(INDEX, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error q = "@t1:@t2:@t3:hello"; - query = new Query(q).dialect(1); - assertTrue("Should contain '@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n'", - client.ftExplain(INDEX, query).contains("@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n")); + query = new Query(q).dialect(1); // dialect=1 should return execution plan + assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString())); query = new Query(q); // dialect=default=2 - try { - client.ftExplain(INDEX, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error q = "@title:{foo}}}}}"; - query = new Query(q).dialect(1); - assertTrue("Should contain 'TAG:@title {\n foo\n}\n'", - client.ftExplain(INDEX, query).contains("TAG:@title {\n foo\n}\n")); + query = new Query(q).dialect(1); // dialect=1 should return execution plan + assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString())); query = new Query(q); // dialect=default=2 - try { - client.ftExplain(INDEX, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error } @Test @@ -194,9 +170,15 @@ public void testAggregationBuilderParamsDialect() { @Test public void dialectBoundSpellCheck() { client.ftCreate(INDEX, TextField.of("t")); - JedisDataException error = Assert.assertThrows(JedisDataException.class, + JedisDataException error = assertThrows(JedisDataException.class, () -> client.ftSpellCheck(INDEX, "Tooni toque kerfuffle", FTSpellCheckParams.spellCheckParams().dialect(0))); - MatcherAssert.assertThat(error.getMessage(), Matchers.containsString("DIALECT requires a non negative integer")); + assertThat(error.getMessage(), containsString("DIALECT requires a non negative integer")); + } + + private void assertSyntaxError(Query query, UnifiedJedis client) { + JedisDataException error = assertThrows(JedisDataException.class, + () -> client.ftExplain(INDEX, query)); + assertThat(error.getMessage(), containsString("Syntax error")); } } diff --git a/src/test/java/redis/clients/jedis/modules/search/SearchTest.java b/src/test/java/redis/clients/jedis/modules/search/SearchTest.java index d6c1739371..661c083631 100644 --- a/src/test/java/redis/clients/jedis/modules/search/SearchTest.java +++ b/src/test/java/redis/clients/jedis/modules/search/SearchTest.java @@ -1,11 +1,14 @@ package redis.clients.jedis.modules.search; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyOrNullString; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.*; import java.util.*; import java.util.stream.Collectors; -import io.redis.test.annotations.SinceRedisVersion; import io.redis.test.utils.RedisVersion; import org.hamcrest.Matchers; import org.junit.Assume; @@ -15,6 +18,7 @@ import org.junit.runners.Parameterized; import redis.clients.jedis.RedisProtocol; +import redis.clients.jedis.UnifiedJedis; import redis.clients.jedis.exceptions.JedisDataException; import redis.clients.jedis.json.Path; import redis.clients.jedis.search.*; @@ -1078,84 +1082,45 @@ public void testDialectsWithFTExplain() throws Exception { String q = "(*)"; Query query = new Query(q).dialect(1); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=1 throws syntax error query = new Query(q).dialect(2); - assertTrue("Should contain 'WILDCARD'", client.ftExplain(index, query).contains("WILDCARD")); + assertThat(client.ftExplain(index, query), containsString("WILDCARD")); q = "$hello"; query = new Query(q).dialect(1); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=1 throws syntax error query = new Query(q).dialect(2).addParam("hello", "hello"); - assertTrue("Should contain 'UNION {\n hello\n +hello(expanded)\n}\n'", - client.ftExplain(index, query).contains("UNION {\n hello\n +hello(expanded)\n}\n")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); q = "@title:(@num:[0 10])"; query = new Query(q).dialect(1); - assertTrue("Should contain 'NUMERIC {0.000000 <= @num <= 10.000000}'", - client.ftExplain(index, query).contains("NUMERIC {0.000000 <= @num <= 10.000000}")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); query = new Query(q).dialect(2); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error q = "@t1:@t2:@t3:hello"; query = new Query(q).dialect(1); - assertTrue("Should contain '@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n'", - client.ftExplain(index, query).contains("@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); query = new Query(q).dialect(2); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error q = "@title:{foo}}}}}"; query = new Query(q).dialect(1); - assertTrue("Should contain 'TAG:@title {\n foo\n}\n'", - client.ftExplain(index, query).contains("TAG:@title {\n foo\n}\n")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); query = new Query(q).dialect(2); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=2 throws syntax error q = "*=>[KNN 10 @v $BLOB]"; query = new Query(q).addParam("BLOB", "aaaa").dialect(1); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=1 throws syntax error query = new Query(q).addParam("BLOB", "aaaa").dialect(2); - assertTrue("Should contain '{K=10 nearest vector'", client.ftExplain(index, query).contains("{K=10 nearest vector")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); - q = "*=>[knn $K @vec_field $BLOB as score]"; + q = "*=>[knn $K @v $BLOB as score]"; query = new Query(q).addParam("BLOB", "aaaa").addParam("K", "10").dialect(1); - try { - client.ftExplain(index, query); - fail(); - } catch (JedisDataException e) { - assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); - } + assertSyntaxError(query, client); // dialect=1 throws syntax error query = new Query(q).addParam("BLOB", "aaaa").addParam("K", "10").dialect(2); - assertTrue("Should contain '{K=10 nearest vector'", client.ftExplain(index, query).contains("{K=10 nearest vector")); + assertThat(client.ftExplain(index, query), not(emptyOrNullString())); } @org.junit.Ignore @@ -1298,4 +1263,10 @@ public void searchIteration() throws Exception { } assertEquals(7, total); } + + void assertSyntaxError(Query query, UnifiedJedis client) { + JedisDataException error = assertThrows(JedisDataException.class, + () -> client.ftExplain(index, query)); + assertThat(error.getMessage(), containsString("Syntax error")); + } } diff --git a/src/test/java/redis/clients/jedis/util/RedisConditions.java b/src/test/java/redis/clients/jedis/util/RedisConditions.java new file mode 100644 index 0000000000..619e8b58a4 --- /dev/null +++ b/src/test/java/redis/clients/jedis/util/RedisConditions.java @@ -0,0 +1,77 @@ +package redis.clients.jedis.util; + +import io.redis.test.utils.RedisVersion; +import redis.clients.jedis.CommandArguments; +import redis.clients.jedis.CommandObject; +import redis.clients.jedis.Module; +import redis.clients.jedis.UnifiedJedis; +import redis.clients.jedis.resps.CommandInfo; + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static redis.clients.jedis.BuilderFactory.MODULE_LIST; +import static redis.clients.jedis.Protocol.Command.COMMAND; +import static redis.clients.jedis.Protocol.Command.MODULE; +import static redis.clients.jedis.Protocol.Keyword.LIST; + +public class RedisConditions { + + private final RedisVersion version; + private final Map modules; + private final Map commands; + + private RedisConditions(RedisVersion version, Map< String, CommandInfo> commands, Map modules) { + this.version = version; + this.commands = commands; + this.modules = modules; + } + + public static RedisConditions of(UnifiedJedis jedis) { + RedisVersion version = RedisVersionUtil.getRedisVersion(jedis); + + CommandObject> commandInfoCmd + = new CommandObject<>(new CommandArguments(COMMAND), CommandInfo.COMMAND_INFO_RESPONSE); + Map commands = jedis.executeCommand(commandInfoCmd); + + CommandObject> moduleListCmd + = new CommandObject<>(new CommandArguments(MODULE).add(LIST), MODULE_LIST); + + Map modules = jedis.executeCommand(moduleListCmd) + .stream() + .collect(Collectors.toMap((m) -> m.getName().toUpperCase(), Module::getVersion)); + + return new RedisConditions(version, commands, modules); + } + + public RedisVersion getVersion() { + return version; + } + + /** + * @param command + * @return {@code true} if the command is present. + */ + public boolean hasCommand(String command) { + return commands.containsKey(command.toUpperCase()); + } + + /** + * @param module + * @return {@code true} if the module is present. + */ + public boolean hasModule(String module) { + return modules.containsKey(module.toUpperCase()); + } + + /** + * @param module + * @param version + * @return {@code true} if the module is present. + */ + public boolean moduleVersionIsGreatherThan(String module, int version) { + Integer moduleVersion = modules.get(module.toUpperCase()); + return moduleVersion != null && moduleVersion > version; + } +}