diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index bdbc3e30cb..f706370bda 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -15,7 +15,12 @@ import { Script, parseInfoResponse, } from "../"; -import { Client, GetAndSetRandomValue, getFirstResult } from "./TestUtilities"; +import { + Client, + GetAndSetRandomValue, + compareMaps, + getFirstResult, +} from "./TestUtilities"; async function getVersion(): Promise<[number, number, number]> { const versionString = await new Promise((resolve, reject) => { @@ -664,10 +669,13 @@ export function runBaseTests(config: { [field2]: value, }; expect(await client.hset(key, fieldValueMap)).toEqual(2); - expect(await client.hgetall(key)).toEqual({ - [field1]: value, - [field2]: value, - }); + + expect( + compareMaps(await client.hgetall(key), { + [field1]: value, + [field2]: value, + }), + ).toBe(true); expect(await client.hgetall("nonExistingKey")).toEqual({}); }, protocol); }, @@ -1602,9 +1610,18 @@ export function runBaseTests(config: { expect(await client.zrange(key, { start: 0, stop: 1 })).toEqual( ["one", "two"], ); + const result = await client.zrangeWithScores(key, { + start: 0, + stop: -1, + }); + expect( - await client.zrangeWithScores(key, { start: 0, stop: -1 }), - ).toEqual({ one: 1.0, two: 2.0, three: 3.0 }); + compareMaps(result, { + one: 1.0, + two: 2.0, + three: 3.0, + }), + ).toBe(true); expect( await client.zrange(key, { start: 0, stop: 1 }, true), ).toEqual(["three", "two"]); @@ -1634,15 +1651,19 @@ export function runBaseTests(config: { type: "byScore", }), ).toEqual(["one", "two"]); + const result = await client.zrangeWithScores(key, { + start: "negativeInfinity", + stop: "positiveInfinity", + type: "byScore", + }); expect( - await client.zrangeWithScores(key, { - start: "negativeInfinity", - stop: "positiveInfinity", - type: "byScore", + compareMaps(result, { + one: 1.0, + two: 2.0, + three: 3.0, }), - ).toEqual({ one: 1.0, two: 2.0, three: 3.0 }); - + ).toBe(true); expect( await client.zrange( key, @@ -1913,10 +1934,13 @@ export function runBaseTests(config: { const membersScores = { a: 1, b: 2, c: 3 }; expect(await client.zadd(key, membersScores)).toEqual(3); expect(await client.zpopmin(key)).toEqual({ a: 1.0 }); - expect(await client.zpopmin(key, 3)).toEqual({ - b: 2.0, - c: 3.0, - }); + + expect( + compareMaps(await client.zpopmin(key, 3), { + b: 2.0, + c: 3.0, + }), + ).toBe(true); expect(await client.zpopmin(key)).toEqual({}); expect(await client.set(key, "value")).toEqual("OK"); await expect(client.zpopmin(key)).rejects.toThrow(); @@ -1934,10 +1958,13 @@ export function runBaseTests(config: { const membersScores = { a: 1, b: 2, c: 3 }; expect(await client.zadd(key, membersScores)).toEqual(3); expect(await client.zpopmax(key)).toEqual({ c: 3.0 }); - expect(await client.zpopmax(key, 3)).toEqual({ - b: 2.0, - a: 1.0, - }); + + expect( + compareMaps(await client.zpopmax(key, 3), { + b: 2.0, + a: 1.0, + }), + ).toBe(true); expect(await client.zpopmax(key)).toEqual({}); expect(await client.set(key, "value")).toEqual("OK"); await expect(client.zpopmax(key)).rejects.toThrow(); diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index 03f5703bf7..d994bde7b4 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -117,6 +117,36 @@ export function parseCommandLineArgs() { return parseArgs(process.argv.slice(2)); } +/** + * Compare two maps by converting them to JSON strings and checking for equality, including property order. + * + * @param map - The first map to compare. + * @param map2 - The second map to compare. + * @returns True if the maps are equal. + * @remarks This function is used to compare maps, including their property order. + * Direct comparison with `expect(map).toEqual(map2)` might ignore the order of properties, + * whereas this function considers property order in the comparison by converting the maps to JSON strings. + * This ensures a stricter comparison that takes property order into account. + * + * @example + * ```typescript + * const mapA = { name: 'John', age: 30 }; + * const mapB = { age: 30, name: 'John' }; + * + * // Direct comparison will pass because it ignores property order + * expect(mapA).toEqual(mapB); // This will pass + * + * // Correct comparison using compareMaps function + * compareMaps(mapA, mapB); // This will return false due to different property order + * ``` + */ +export function compareMaps( + map: Record, + map2: Record, +): boolean { + return JSON.stringify(map) == JSON.stringify(map2); +} + export async function transactionTest( baseTransaction: Transaction | ClusterTransaction, ): Promise { diff --git a/node/tests/UtilsTests.test.ts b/node/tests/UtilsTests.test.ts new file mode 100644 index 0000000000..b5533ddaa5 --- /dev/null +++ b/node/tests/UtilsTests.test.ts @@ -0,0 +1,104 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ + +import { describe, expect, it } from "@jest/globals"; +import { compareMaps } from "./TestUtilities"; + +describe("test compareMaps", () => { + it("Map comparison of empty maps should return true", () => { + const map1 = {}; + const map2 = {}; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with the same key-value pairs should return true", () => { + const map1 = { a: 1, b: 2 }; + const map2 = { a: 1, b: 2 }; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with different key-value pairs should return false", () => { + const map1 = { a: 1, b: 2 }; + const map2 = { a: 1, b: 3 }; + expect(compareMaps(map1, map2)).toBe(false); + }); + + it("Map comparison of maps with different key-value pairs order should return false", () => { + const map1 = { a: 1, b: 2 }; + const map2 = { b: 2, a: 1 }; + expect(compareMaps(map1, map2)).toBe(false); + }); + + it("Map comparison of maps with nested maps having the same values should return true", () => { + const map1 = { a: { b: 1 } }; + const map2 = { a: { b: 1 } }; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with nested maps having different values should return false", () => { + const map1 = { a: { b: 1 } }; + const map2 = { a: { b: 2 } }; + expect(compareMaps(map1, map2)).toBe(false); + }); + + it("Map comparison of maps with nested maps having different order should return false", () => { + const map1 = { a: { b: 1, c: 2 } }; + const map2 = { a: { c: 2, b: 1 } }; + expect(compareMaps(map1, map2)).toBe(false); + }); + + it("Map comparison of maps with arrays as values having the same values should return true", () => { + const map1 = { a: [1, 2] }; + const map2 = { a: [1, 2] }; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with arrays as values having different values should return false", () => { + const map1 = { a: [1, 2] }; + const map2 = { a: [1, 3] }; + expect(compareMaps(map1, map2)).toBe(false); + }); + + it("Map comparison of maps with null values should return true", () => { + const map1 = { a: null }; + const map2 = { a: null }; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with mixed types of values should return true", () => { + const map1 = { + a: 1, + b: { c: [2, 3] }, + d: null, + e: "string", + f: [1, "2", true], + }; + const map2 = { + a: 1, + b: { c: [2, 3] }, + d: null, + e: "string", + f: [1, "2", true], + }; + expect(compareMaps(map1, map2)).toBe(true); + }); + + it("Map comparison of maps with mixed types of values should return false", () => { + const map1 = { + a: 1, + b: { c: [2, 3] }, + d: null, + e: "string", + f: [1, "2", false], + }; + const map2 = { + a: 1, + b: { c: [2, 3] }, + d: null, + f: [1, "2", false], + e: "string", + }; + expect(compareMaps(map1, map2)).toBe(false); + }); +}); diff --git a/python/python/glide/constants.py b/python/python/glide/constants.py index 54232dd6c5..6c2cf47148 100644 --- a/python/python/glide/constants.py +++ b/python/python/glide/constants.py @@ -1,6 +1,6 @@ # Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 -from typing import Dict, List, Literal, Optional, Set, TypeVar, Union +from typing import Dict, List, Literal, Mapping, Optional, Set, TypeVar, Union from glide.protobuf.connection_request_pb2 import ConnectionRequest from glide.protobuf.redis_request_pb2 import RedisRequest @@ -18,6 +18,7 @@ int, None, Dict[str, T], + Mapping[str, "TResult"], float, Set[T], List[T], diff --git a/python/python/tests/test_async_client.py b/python/python/tests/test_async_client.py index 99e9adf63e..0461e8132a 100644 --- a/python/python/tests/test_async_client.py +++ b/python/python/tests/test_async_client.py @@ -4,14 +4,12 @@ import asyncio import math -import random -import string import time from datetime import datetime, timedelta, timezone -from typing import Dict, List, TypeVar, Union, cast +from typing import Dict, Union, cast import pytest -from glide import ClosingError, RequestError, Script, TimeoutError +from glide import ClosingError, RequestError, Script from glide.async_commands.core import ( ConditionalChange, ExpireOptions, @@ -49,75 +47,15 @@ SlotKeyRoute, SlotType, ) -from packaging import version from tests.conftest import create_client - -T = TypeVar("T") - - -def is_single_response(response: T, single_res: T) -> bool: - """ - Recursively checks if a given response matches the type structure of single_res. - - Args: - response (T): The response to check. - single_res (T): An object with the expected type structure as an example for the single node response. - - Returns: - bool: True if response matches the structure of single_res, False otherwise. - - Example: - >>> is_single_response(["value"], LIST_STR) - True - >>> is_single_response([["value"]], LIST_STR) - False - """ - if isinstance(single_res, list) and isinstance(response, list): - return is_single_response(response[0], single_res[0]) - elif isinstance(response, type(single_res)): - return True - return False - - -def get_first_result(res: str | List[str] | List[List[str]] | Dict[str, str]) -> str: - while isinstance(res, list): - res = ( - res[1] - if not isinstance(res[0], list) and res[0].startswith("127.0.0.1") - else res[0] - ) - - if isinstance(res, dict): - res = list(res.values())[0] - - return res - - -def parse_info_response(res: str | Dict[str, str]) -> Dict[str, str]: - res = get_first_result(res) - info_lines = [ - line for line in res.splitlines() if line and not line.startswith("#") - ] - info_dict = {} - for line in info_lines: - splitted_line = line.split(":") - key = splitted_line[0] - value = splitted_line[1] - info_dict[key] = value - return info_dict - - -def get_random_string(length): - result_str = "".join(random.choice(string.ascii_letters) for i in range(length)) - return result_str - - -async def check_if_server_version_lt(client: TRedisClient, min_version: str) -> bool: - # TODO: change it to pytest fixture after we'll implement a sync client - info_str = await client.info([InfoSection.SERVER]) - redis_version = parse_info_response(info_str).get("redis_version") - assert redis_version is not None - return version.parse(redis_version) < version.parse(min_version) +from tests.utils.utils import ( + check_if_server_version_lt, + compare_maps, + get_first_result, + get_random_string, + is_single_response, + parse_info_response, +) @pytest.mark.asyncio @@ -634,7 +572,9 @@ async def test_hset_hget_hgetall(self, redis_client: TRedisClient): assert await redis_client.hget(key, field2) == "value2" assert await redis_client.hget(key, "non_existing_field") is None - assert await redis_client.hgetall(key) == {field: "value", field2: "value2"} + hgetall_map = await redis_client.hgetall(key) + expected_map = {field: "value", field2: "value2"} + assert compare_maps(hgetall_map, expected_map) is True assert await redis_client.hgetall("non_existing_field") == {} @pytest.mark.parametrize("cluster_mode", [True, False]) @@ -1882,7 +1822,10 @@ async def test_zpopmin(self, redis_client: TRedisClient): assert await redis_client.zadd(key, members_scores=members_scores) == 3 assert await redis_client.zpopmin(key) == {"a": 1.0} - assert await redis_client.zpopmin(key, 3) == {"b": 2.0, "c": 3.0} + zpopmin_map = await redis_client.zpopmin(key, 3) + expected_map = {"b": 2.0, "c": 3.0} + assert compare_maps(zpopmin_map, expected_map) is True + assert await redis_client.zpopmin(key) == {} assert await redis_client.set(key, "value") == OK with pytest.raises(RequestError): @@ -1897,7 +1840,11 @@ async def test_zpopmax(self, redis_client: TRedisClient): members_scores = {"a": 1.0, "b": 2.0, "c": 3.0} assert await redis_client.zadd(key, members_scores) == 3 assert await redis_client.zpopmax(key) == {"c": 3.0} - assert await redis_client.zpopmax(key, 3) == {"b": 2.0, "a": 1.0} + + zpopmax_map = await redis_client.zpopmax(key, 3) + expected_map = {"b": 2.0, "a": 1.0} + assert compare_maps(zpopmax_map, expected_map) is True + assert await redis_client.zpopmax(key) == {} assert await redis_client.set(key, "value") == OK with pytest.raises(RequestError): @@ -1917,9 +1864,11 @@ async def test_zrange_by_index(self, redis_client: TRedisClient): "two", ] - assert ( - await redis_client.zrange_withscores(key, RangeByIndex(start=0, stop=-1)) - ) == {"one": 1.0, "two": 2.0, "three": 3.0} + zrange_map = await redis_client.zrange_withscores( + key, RangeByIndex(start=0, stop=-1) + ) + expected_map = {"one": 1.0, "two": 2.0, "three": 3.0} + assert compare_maps(zrange_map, expected_map) is True assert await redis_client.zrange( key, RangeByIndex(start=0, stop=1), reverse=True @@ -1948,12 +1897,12 @@ async def test_zrange_byscore(self, redis_client: TRedisClient): ), ) == ["one", "two"] - assert ( - await redis_client.zrange_withscores( - key, - RangeByScore(start=InfBound.NEG_INF, stop=InfBound.POS_INF), - ) - ) == {"one": 1.0, "two": 2.0, "three": 3.0} + zrange_map = await redis_client.zrange_withscores( + key, + RangeByScore(start=InfBound.NEG_INF, stop=InfBound.POS_INF), + ) + expected_map = {"one": 1.0, "two": 2.0, "three": 3.0} + assert compare_maps(zrange_map, expected_map) is True assert await redis_client.zrange( key, diff --git a/python/python/tests/test_transaction.py b/python/python/tests/test_transaction.py index a513ac520d..df45b3b8e4 100644 --- a/python/python/tests/test_transaction.py +++ b/python/python/tests/test_transaction.py @@ -26,7 +26,7 @@ from glide.constants import OK, TResult from glide.redis_client import RedisClient, RedisClusterClient, TRedisClient from tests.conftest import create_client -from tests.test_async_client import check_if_server_version_lt, get_random_string +from tests.utils.utils import check_if_server_version_lt, get_random_string async def transaction_test( diff --git a/python/python/tests/test_utils.py b/python/python/tests/test_utils.py index ca6c75300b..25e50e53eb 100644 --- a/python/python/tests/test_utils.py +++ b/python/python/tests/test_utils.py @@ -1,7 +1,9 @@ # Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 +import pytest from glide.logger import Level, Logger from tests.conftest import DEFAULT_TEST_LOG_LEVEL +from tests.utils.utils import compare_maps class TestLogger: @@ -16,3 +18,89 @@ def test_set_logger_config(self): # Revert to the tests default log level Logger.set_logger_config(DEFAULT_TEST_LOG_LEVEL) assert Logger.logger_level == DEFAULT_TEST_LOG_LEVEL.value + + +class TestCompareMaps: + def test_empty_maps(self): + map1 = {} + map2 = {} + assert compare_maps(map1, map2) is True + + def test_same_key_value_pairs(self): + map1 = {"a": 1, "b": 2} + map2 = {"a": 1, "b": 2} + assert compare_maps(map1, map2) is True + + def test_different_key_value_pairs(self): + map1 = {"a": 1, "b": 2} + map2 = {"a": 1, "b": 3} + assert compare_maps(map1, map2) is False + + def test_different_key_value_pairs_order(self): + map1 = {"a": 1, "b": 2} + map2 = {"b": 2, "a": 1} + assert compare_maps(map1, map2) is False + + def test_nested_maps_same_values(self): + map1 = {"a": {"b": 1}} + map2 = {"a": {"b": 1}} + assert compare_maps(map1, map2) is True + + def test_nested_maps_different_values(self): + map1 = {"a": {"b": 1}} + map2 = {"a": {"b": 2}} + assert compare_maps(map1, map2) is False + + def test_nested_maps_different_order(self): + map1 = {"a": {"b": 1, "c": 2}} + map2 = {"a": {"c": 2, "b": 1}} + assert compare_maps(map1, map2) is False + + def test_arrays_same_values(self): + map1 = {"a": [1, 2]} + map2 = {"a": [1, 2]} + assert compare_maps(map1, map2) is True + + def test_arrays_different_values(self): + map1 = {"a": [1, 2]} + map2 = {"a": [1, 3]} + assert compare_maps(map1, map2) is False + + def test_null_values(self): + map1 = {"a": None} + map2 = {"a": None} + assert compare_maps(map1, map2) is True + + def test_mixed_types_same_values(self): + map1 = { + "a": 1, + "b": {"c": [2, 3]}, + "d": None, + "e": "string", + "f": [1, "2", True], + } + map2 = { + "a": 1, + "b": {"c": [2, 3]}, + "d": None, + "e": "string", + "f": [1, "2", True], + } + assert compare_maps(map1, map2) is True + + def test_mixed_types_different_values(self): + map1 = { + "a": 1, + "b": {"c": [2, 3]}, + "d": None, + "e": "string", + "f": [1, "2", False], + } + map2 = { + "a": 1, + "b": {"c": [2, 3]}, + "d": None, + "f": [1, "2", False], + "e": "string", + } + assert compare_maps(map1, map2) is False diff --git a/python/python/tests/utils/utils.py b/python/python/tests/utils/utils.py new file mode 100644 index 0000000000..c0996a45bf --- /dev/null +++ b/python/python/tests/utils/utils.py @@ -0,0 +1,111 @@ +import json +import random +import string +from typing import Any, Dict, List, Mapping, TypeVar, Union + +from glide.async_commands.core import InfoSection +from glide.constants import TResult +from glide.redis_client import TRedisClient +from packaging import version + +T = TypeVar("T") + + +def is_single_response(response: T, single_res: T) -> bool: + """ + Recursively checks if a given response matches the type structure of single_res. + + Args: + response (T): The response to check. + single_res (T): An object with the expected type structure as an example for the single node response. + + Returns: + bool: True if response matches the structure of single_res, False otherwise. + + Example: + >>> is_single_response(["value"], LIST_STR) + True + >>> is_single_response([["value"]], LIST_STR) + False + """ + if isinstance(single_res, list) and isinstance(response, list): + return is_single_response(response[0], single_res[0]) + elif isinstance(response, type(single_res)): + return True + return False + + +def get_first_result( + res: Union[str, List[str], List[List[str]], Dict[str, str]] +) -> str: + while isinstance(res, list): + res = ( + res[1] + if not isinstance(res[0], list) and res[0].startswith("127.0.0.1") + else res[0] + ) + + if isinstance(res, dict): + res = list(res.values())[0] + + return res + + +def parse_info_response(res: Union[str, Dict[str, str]]) -> Dict[str, str]: + res = get_first_result(res) + info_lines = [ + line for line in res.splitlines() if line and not line.startswith("#") + ] + info_dict = {} + for line in info_lines: + splitted_line = line.split(":") + key = splitted_line[0] + value = splitted_line[1] + info_dict[key] = value + return info_dict + + +def get_random_string(length): + result_str = "".join(random.choice(string.ascii_letters) for i in range(length)) + return result_str + + +async def check_if_server_version_lt(client: TRedisClient, min_version: str) -> bool: + # TODO: change it to pytest fixture after we'll implement a sync client + info_str = await client.info([InfoSection.SERVER]) + redis_version = parse_info_response(info_str).get("redis_version") + assert redis_version is not None + return version.parse(redis_version) < version.parse(min_version) + + +def compare_maps( + map: Union[Mapping[str, TResult], Dict[str, TResult]], + map2: Union[Mapping[str, TResult], Dict[str, TResult]], +) -> bool: + """ + Compare two maps by converting them to JSON strings and checking for equality, including property order. + + Args: + map (Union[Mapping[str, TResult], Dict[str, TResult]]): The first map to compare. + map2 (Union[Mapping[str, TResult], Dict[str, TResult]]): The second map to compare. + + Returns: + bool: True if the maps are equal, False otherwise. + + Notes: + This function compares two maps, including their property order. + It checks that each key-value pair in `map1` is equal to the corresponding key-value pair in `map2`, + and ensures that the order of properties is also the same. + Direct comparison with `assert map == map2` might ignore the order of properties. + + Example: + mapA = {'name': 'John', 'age': 30} + mapB = {'age': 30, 'name': 'John'} + + # Direct comparison will pass because it ignores property order + assert mapA == mapB # This will pass + + # Correct comparison using compare_maps function + compare_maps(mapA, mapB) # This will return False due to different property order + """ + return json.dumps(map) == json.dumps(map2)