Skip to content

Commit

Permalink
assert: make partialDeepStrictEqual throw when comparing [0] with [-0]
Browse files Browse the repository at this point in the history
Fixes: #56230
  • Loading branch information
puskin94 committed Dec 12, 2024
1 parent a1d980c commit 65e1141
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 20 deletions.
79 changes: 59 additions & 20 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,17 @@ function partiallyCompareSets(actual, expected, comparedObjects) {
return true;
}

function isZeroOrMinusZero(item) {
return ObjectIs(item, -0) || ObjectIs(item, 0);
}

// Helper function to get a unique key for 0, -0 to avoid collisions
function getZeroKey(item) {
if (ObjectIs(item, 0)) return '0';
if (ObjectIs(item, -0)) return '-0';
return item;
}

function partiallyCompareArrays(actual, expected, comparedObjects) {
if (expected.length > actual.length) {
return false;
Expand All @@ -507,38 +518,66 @@ function partiallyCompareArrays(actual, expected, comparedObjects) {
// Create a map to count occurrences of each element in the expected array
const expectedCounts = new SafeMap();
for (const expectedItem of expected) {
let found = false;
for (const { 0: key, 1: count } of expectedCounts) {
if (isDeepStrictEqual(key, expectedItem)) {
expectedCounts.set(key, count + 1);
found = true;
break;
// Check if the item is a zero or a -0, as these need to be handled separately
if (isZeroOrMinusZero(expectedItem)) {
const zeroKey = getZeroKey(expectedItem);
expectedCounts.set(zeroKey, {
count: (expectedCounts.get(zeroKey)?.count || 0) + 1,
expectedType: typeof expectedItem,
});
} else {
let found = false;
for (const { 0: key, 1: { count, expectedType } } of expectedCounts) {
// This is a very ugly solution to not make eslint valid-typeof rule whine
const haveSameType = expectedType === (typeof expectedItem === 'number' ? 'number' : typeof expectedItem);
if (isDeepStrictEqual(key, expectedItem) && haveSameType) {
expectedCounts.set(key, { count: count + 1, expectedType });
found = true;
break;
}
}
if (!found) {
expectedCounts.set(expectedItem, { count: 1, expectedType: typeof expectedItem });
}
}
if (!found) {
expectedCounts.set(expectedItem, 1);
}
}

const safeActual = new SafeArrayIterator(actual);

// Create a map to count occurrences of relevant elements in the actual array
for (const actualItem of safeActual) {
for (const { 0: key, 1: count } of expectedCounts) {
if (isDeepStrictEqual(key, actualItem)) {
if (count === 1) {
expectedCounts.delete(key);
} else {
expectedCounts.set(key, count - 1);
// Check if the item is a zero or a -0, as these need to be handled separately
if (isZeroOrMinusZero(actualItem)) {
const zeroKey = getZeroKey(actualItem);

if (expectedCounts.has(zeroKey)) {
const { count, expectedType } = expectedCounts.get(zeroKey);
// This is a very ugly solution to not make eslint valid-typeof rule whine
const haveSameType = expectedType === (typeof actualItem === 'number' ? 'number' : typeof actualItem);
if (haveSameType) {
if (count === 1) {
expectedCounts.delete(zeroKey);
} else {
expectedCounts.set(zeroKey, { count: count - 1, expectedType });
}
}
}
} else {
for (const { 0: expectedItem, 1: { count, expectedType } } of expectedCounts) {
// This is a very ugly solution to not make eslint valid-typeof rule whine
const haveSameType = expectedType === (typeof actualItem === 'number' ? 'number' : typeof actualItem);
if (isDeepStrictEqual(expectedItem, actualItem) && haveSameType) {
if (count === 1) {
expectedCounts.delete(expectedItem);
} else {
expectedCounts.set(expectedItem, { count: count - 1, expectedType });
}
break;
}
break;
}
}
}

const { size } = expectedCounts;
expectedCounts.clear();
return size === 0;
return expectedCounts.size === 0;
}

/**
Expand Down
6 changes: 6 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const assert = require('assert');

const actual = ['-0'];
const expected = [-0];

assert.partialDeepStrictEqual(actual, expected);
50 changes: 50 additions & 0 deletions test/parallel/test-assert-objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,41 @@ describe('Object Comparison Tests', () => {
actual: [1, 'two', true],
expected: [1, 'two', false],
},
{
description: 'throws when comparing [0] with [-0]',
actual: [0],
expected: [-0],
},
{
description: 'throws when comparing [0, 0, 0] with [0, -0]',
actual: [0, 0, 0],
expected: [0, -0],
},
{
description: 'throws when comparing [-0] with [0]',
actual: [0],
expected: [-0],
},
{
description: 'throws when comparing ["-0"] with [-0]',
actual: ['-0'],
expected: [-0],
},
{
description: 'throws when comparing [-0] with ["-0"]',
actual: [-0],
expected: ['-0'],
},
{
description: 'throws when comparing ["0"] with [0]',
actual: ['0'],
expected: [0],
},
{
description: 'throws when comparing [0] with ["0"]',
actual: [0],
expected: ['0'],
},
{
description:
'throws when comparing two Date objects with different times',
Expand Down Expand Up @@ -385,6 +420,21 @@ describe('Object Comparison Tests', () => {
actual: [1, 'two', true],
expected: [1, 'two', true],
},
{
description: 'compares [0] with [0]',
actual: [0],
expected: [0],
},
{
description: 'compares [-0] with [-0]',
actual: [-0],
expected: [-0],
},
{
description: 'compares [0, -0, 0] with [0, 0]',
actual: [0, -0, 0],
expected: [0, 0],
},
{
description: 'compares two Date objects with the same time',
actual: new Date(0),
Expand Down

0 comments on commit 65e1141

Please sign in to comment.