Skip to content

Fix dictionary key order sensitivity in Spector tests for languages with non-deterministic ordering #7866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/spec-api/src/request-validations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,22 @@ export const validateRawBodyEquals = (
return;
}

// For JSON content, perform semantic comparison to handle dictionary key order differences
if (typeof actualRawBody === "string" && typeof expectedRawBody === "string") {
if (isJsonContent(actualRawBody) && isJsonContent(expectedRawBody)) {
try {
const actualParsed = JSON.parse(actualRawBody);
const expectedParsed = JSON.parse(expectedRawBody);
if (!deepEqual(actualParsed, expectedParsed, { strict: true })) {
throw new ValidationError(BODY_NOT_EQUAL_ERROR_MESSAGE, expectedRawBody, actualRawBody);
}
return;
} catch {
// If JSON parsing fails, fall back to string comparison
}
}
}

if (!deepEqual(actualRawBody, expectedRawBody, { strict: true })) {
throw new ValidationError(BODY_NOT_EQUAL_ERROR_MESSAGE, expectedRawBody, actualRawBody);
}
Expand Down Expand Up @@ -122,6 +138,18 @@ const isBodyEmpty = (body: string | Buffer | undefined | null) => {
return body == null || body === "" || body.length === 0;
};

/**
* Check if the provided string content appears to be JSON.
* @param content string content to check.
*/
const isJsonContent = (content: string): boolean => {
const trimmed = content.trim();
return (
(trimmed.startsWith("{") && trimmed.endsWith("}")) ||
(trimmed.startsWith("[") && trimmed.endsWith("]"))
);
};

/**
* Check whether the request header contains the given name/value pair
*/
Expand Down
202 changes: 202 additions & 0 deletions packages/spec-api/test/dictionary-order.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
import { describe, expect, it } from "vitest";
import { RequestExpectation } from "../src/expectation.js";
import { RequestExt } from "../src/types.js";
import { validateBodyEquals, validateCoercedDateBodyEquals, validateRawBodyEquals } from "../src/request-validations.js";

describe("Dictionary order sensitivity", () => {
it("should demonstrate current issue with string serialization order", () => {
// Test what actually happens when we serialize dictionaries
const dict1 = { k2: 2, k1: 1 };
const dict2 = { k1: 1, k2: 2 };

const str1 = JSON.stringify(dict1);
const str2 = JSON.stringify(dict2);

console.log("Dictionary 1 string:", str1); // {"k2":2,"k1":1}
console.log("Dictionary 2 string:", str2); // {"k1":1,"k2":2}

// They create different JSON strings due to key order
expect(str1).not.toEqual(str2);
});

it("should now handle raw body comparison with different key order", () => {
// This used to be the issue - now it should pass
const requestExt = {
body: { k2: 2, k1: 1 },
rawBody: '{"k2":2,"k1":1}',
} as RequestExt;

// Expected raw body with different key order
const expectedRawBody = '{"k1":1,"k2":2}';

// This now passes due to semantic JSON comparison
expect(() => validateRawBodyEquals(requestExt, expectedRawBody)).not.toThrow();
});

it("should handle dictionaries with different key orders using direct validation", () => {
// Simulate request with dictionary in one order where body is parsed JSON
const requestExt = {
body: JSON.parse('{"k2":2,"k1":1}'), // This creates object with different internal order potentially
rawBody: '{"k2":2,"k1":1}',
} as RequestExt;

// Expected dictionary in different order
const expectedBody = { k1: 1, k2: 2 };

// Test the actual validation function directly
expect(() => validateBodyEquals(requestExt, expectedBody)).not.toThrow();
});

it("should handle dictionaries with different key orders", () => {
// Simulate request with dictionary in one order
const requestExt = {
body: { k2: 2, k1: 1 },
rawBody: JSON.stringify({ k2: 2, k1: 1 }),
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected dictionary in different order
const expectedBody = { k1: 1, k2: 2 };

// This should pass - bodyEquals uses parsed body comparison which handles order
expect(() => requestExpectation.bodyEquals(expectedBody)).not.toThrow();
});

it("should now handle rawBodyEquals with different key order", () => {
// Simulate request with dictionary in one order
const requestExt = {
body: { k2: 2, k1: 1 },
rawBody: '{"k2":2,"k1":1}',
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected raw body string with different key order
const expectedRawBody = '{"k1":1,"k2":2}';

// This now passes due to semantic JSON comparison
expect(() => requestExpectation.rawBodyEquals(expectedRawBody)).not.toThrow();
});

it("should handle nested dictionaries with different key orders", () => {
// Simulate request with nested dictionary in one order
const requestExt = {
body: {
outer: { k2: 2, k1: 1 },
data: "test"
},
rawBody: JSON.stringify({
outer: { k2: 2, k1: 1 },
data: "test"
}),
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected nested dictionary in different order
const expectedBody = {
data: "test",
outer: { k1: 1, k2: 2 }
};

// This should pass
expect(() => requestExpectation.bodyEquals(expectedBody)).not.toThrow();
});

it("should handle coerced dictionaries with different key orders", () => {
// Simulate request with dictionary in one order
const requestExt = {
body: { k2: "2022-08-26T18:38:00.000Z", k1: "2022-08-26T18:38:00.000Z" },
rawBody: JSON.stringify({ k2: "2022-08-26T18:38:00.000Z", k1: "2022-08-26T18:38:00.000Z" }),
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected dictionary in different order with coerced dates
const expectedBody = { k1: "2022-08-26T18:38:00Z", k2: "2022-08-26T18:38:00Z" };

// This should pass
expect(() => requestExpectation.coercedBodyEquals(expectedBody)).not.toThrow();
});

it("should still fail for semantically different dictionaries", () => {
// Simulate request with dictionary
const requestExt = {
body: { k1: 1, k2: 2 },
rawBody: JSON.stringify({ k1: 1, k2: 2 }),
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected dictionary with different values
const expectedBody = { k1: 1, k2: 3 };

// This should still fail because the values are actually different
expect(() => requestExpectation.bodyEquals(expectedBody)).toThrow();
});

it("should still fail for dictionaries with different keys", () => {
// Simulate request with dictionary
const requestExt = {
body: { k1: 1, k2: 2 },
rawBody: JSON.stringify({ k1: 1, k2: 2 }),
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected dictionary with different keys
const expectedBody = { k1: 1, k3: 2 };

// This should still fail because the keys are actually different
expect(() => requestExpectation.bodyEquals(expectedBody)).toThrow();
});

it("should handle non-JSON content normally", () => {
// Test non-JSON raw body comparison
const requestExt = {
body: "plain text content",
rawBody: "plain text content",
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected different plain text
const expectedRawBody = "different plain text";

// This should still fail for different plain text content
expect(() => requestExpectation.rawBodyEquals(expectedRawBody)).toThrow();
});

it("should handle invalid JSON gracefully", () => {
// Test invalid JSON that can't be parsed
const requestExt = {
body: '{"invalid": json}',
rawBody: '{"invalid": json}',
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected different invalid JSON
const expectedRawBody = '{"different": invalid}';

// This should fall back to string comparison and fail
expect(() => requestExpectation.rawBodyEquals(expectedRawBody)).toThrow();
});

it("should still pass for identical non-JSON content", () => {
// Test identical non-JSON raw body comparison
const requestExt = {
body: "identical text content",
rawBody: "identical text content",
} as RequestExt;

const requestExpectation = new RequestExpectation(requestExt);

// Expected identical plain text
const expectedRawBody = "identical text content";

// This should pass for identical content
expect(() => requestExpectation.rawBodyEquals(expectedRawBody)).not.toThrow();
});
});