Commit a9ce9227 authored by Robert Knight's avatar Robert Knight

Remove a test which doesn't test what it intends to

Dates are in fact JSON-serializable, but as strings.

Recent changes to `isMessageEqual` mean that the `JSON.stringify` calls
cannot fail, because the `isMessage` check ensures all the fields we
serialize are strings. Writing a test to check for this failure mode is
not possible. There is a refactoring hazard here, so I added a comment
to call this out.
parent 28fa4e8c
...@@ -42,14 +42,12 @@ export function isMessageEqual(data, message) { ...@@ -42,14 +42,12 @@ export function isMessageEqual(data, message) {
return false; return false;
} }
try { // We assume `JSON.stringify` cannot throw because `isMessage` verifies that
// all the fields we serialize here are serializable values.
return ( return (
JSON.stringify(data, Object.keys(message).sort()) === JSON.stringify(data, Object.keys(message).sort()) ===
JSON.stringify(message, Object.keys(message).sort()) JSON.stringify(message, Object.keys(message).sort())
); );
} catch {
return false;
}
} }
/** /**
......
...@@ -95,15 +95,6 @@ describe('port-util', () => { ...@@ -95,15 +95,6 @@ describe('port-util', () => {
expectedResult: false, expectedResult: false,
reason: 'data has one property that is different', reason: 'data has one property that is different',
}, },
{
data: {
frame1: new Date(), // not JSON-serializable
frame2,
type,
},
expectedResult: false,
reason: "data has one property that can't be serialized",
},
].forEach(({ data, expectedResult, reason }) => { ].forEach(({ data, expectedResult, reason }) => {
it(`returns '${expectedResult}' because the ${reason}`, () => { it(`returns '${expectedResult}' because the ${reason}`, () => {
const result = isMessageEqual(data, { const result = isMessageEqual(data, {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment