Refactor map comparison in Python and Node (valkey-io#1319)

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<string>((resolve, reject) => {
@@ -664,10 +669,13 @@ export function runBaseTests<Context>(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<Context>(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<Context>(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<Context>(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<Context>(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();

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<string, unknown>,
+    map2: Record<string, unknown>,
+): boolean {
+    return JSON.stringify(map) == JSON.stringify(map2);
+}
+
 export async function transactionTest(
     baseTransaction: Transaction | ClusterTransaction,
 ): Promise<ReturnType[]> {

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);
+    });
+});

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],