Skip to content
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.

Sign up for GitHub

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

Fixes Deep Compare #35

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+90 −37
Open

Fixes Deep Compare #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5a3508
fix(assert): undefined error on deep compare via objectContain and no…
azachar May 7, 2021
623587c
refactor(test): being specific about the expect error
azachar May 7, 2021
1239a7d
fix(assert): handling of empty vs undefined arrays when deep compare
azachar May 7, 2021
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
23 changes: 16 additions & 7 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,17 @@ assert.notImplement = (subject, methods) => {

assert.objectContent = (obj, content) => {
Object.keys(content).forEach(key => {
if (isObject(content[key])) {
assert.objectContent(obj[key], content[key]);
let actualValue = obj[key];
if (actualValue && isObject(content[key])) {
assert.objectContent(actualValue, content[key]);
return;
}

assert.equal(obj[key], content[key]);
if (Array.isArray(content[key]) && !content[key].length && actualValue === undefined) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& actualValue === undefined

No tests fail if this is removed - both here and later on. Either there's some missing unit test to be added or these checks aren't necessary and can be removed. Either way, could you check and make the right change please?

// undefined array and empty array are ok
return;
}
assert.equal(actualValue, content[key]);
});
};

Expand All @@ -244,12 +249,16 @@ assert.objectContent = (obj, content) => {

assert.noObjectContent = (obj, content) => {
Object.keys(content).forEach(key => {
if (isObject(content[key])) {
assert.noObjectContent(obj[key], content[key]);
let actualValue = obj[key]
if (actualValue && isObject(content[key])) {
assert.noObjectContent(actualValue, content[key]);
return;
}

assert.notEqual(obj[key], content[key]);
if (Array.isArray(content[key]) && !content[key].length && actualValue === undefined) {
// undefined array should be not empty
assert.fail(`${key} is expected to be not empty`)
}
assert.notEqual(actualValue, content[key]);
});
};

Expand Down
104 changes: 74 additions & 30 deletions test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ describe('yeoman-assert', () => {

it('reject a file that does not exist', () => {
assert.throws(yoAssert.file.bind(yoAssert, 'etherealTestFile'));
});
}, /.*AssertionError.*/);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I'd revert these. I don't plan on blocking on this, but just stating a preference: I don't think these are worth adding. Nothing prevents the comments from getting out of date. And we already know some error is being thrown. If we really wanted to assert on which error that is, we can always check its class/constructor/name.

Totally up to you though, feel free to ignore this suggestion. 🙂


it('reject multiple files one of which does not exist', () => {
assert.throws(yoAssert.file.bind(yoAssert, ['testFile', 'intangibleTestFile']));
});
}, /.*AssertionError.*/);
});

describe('.noFile()', () => {
Expand All @@ -44,12 +44,12 @@ describe('yeoman-assert', () => {

it('reject a file that exists', () => {
assert.throws(yoAssert.noFile.bind(yoAssert, 'testFile'));
});
}, /.*AssertionError.*/);

it('reject an array of files one of which exists', () => {
assert.throws(
yoAssert.noFile.bind(yoAssert, ['testFile', 'etherealTestFile']));
});
}, /.*AssertionError.*/);
});

describe('.fileContent()', () => {
Expand All @@ -62,11 +62,11 @@ describe('yeoman-assert', () => {
});

it('reject a file and regex when the file content does not match the regex', () => {
assert.throws(yoAssert.fileContent.bind(yoAssert, 'testFile', /Roses are blue/));
assert.throws(yoAssert.fileContent.bind(yoAssert, 'testFile', /Roses are blue/), /.*AssertionError.*/);
});

it('reject a file and string when the file content does not contain the string', () => {
assert.throws(yoAssert.fileContent.bind(yoAssert, 'testFile', 'Roses are blue'));
assert.throws(yoAssert.fileContent.bind(yoAssert, 'testFile', 'Roses are blue'), /.*AssertionError.*/);
});

it('accept an array of file/regex pairs when each file\'s content matches the corresponding regex', () => {
Expand All @@ -82,7 +82,7 @@ describe('yeoman-assert', () => {
['testFile', /Roses are red/],
['testFile2', /Violets are orange/]
];
assert.throws(yoAssert.fileContent.bind(yoAssert, arg));
assert.throws(yoAssert.fileContent.bind(yoAssert, arg), /.*AssertionError.*/);
});
});

Expand All @@ -92,7 +92,7 @@ describe('yeoman-assert', () => {
});

it('reject a file and string when the file content does not equal the string', () => {
assert.throws(yoAssert.equalsFileContent.bind(yoAssert, 'testFile', 'Roses are red'));
assert.throws(yoAssert.equalsFileContent.bind(yoAssert, 'testFile', 'Roses are red'), /.*AssertionError.*/);
});

it('accept an array of file/string pairs when each file\'s content equals the corresponding string', () => {
Expand All @@ -108,7 +108,7 @@ describe('yeoman-assert', () => {
['testFile', 'Roses are red.\n'],
['testFile2', 'Violets are green.\n']
];
assert.throws(yoAssert.equalsFileContent.bind(yoAssert, arg));
assert.throws(yoAssert.equalsFileContent.bind(yoAssert, arg), /.*AssertionError.*/);
});
});

Expand All @@ -122,11 +122,11 @@ describe('yeoman-assert', () => {
});

it('reject a file and regex when the file content matches the regex', () => {
assert.throws(yoAssert.noFileContent.bind(yoAssert, 'testFile', /Roses are red/));
assert.throws(yoAssert.noFileContent.bind(yoAssert, 'testFile', /Roses are red/), /.*AssertionError.*/);
});

it('reject a file and string when the file content contain the string', () => {
assert.throws(yoAssert.noFileContent.bind(yoAssert, 'testFile', 'Roses are red'));
assert.throws(yoAssert.noFileContent.bind(yoAssert, 'testFile', 'Roses are red'), /.*AssertionError.*/);
});

it('accept an array of file/regex pairs when each file\'s content does not match its corresponding regex', () => {
Expand All @@ -142,7 +142,7 @@ describe('yeoman-assert', () => {
['testFile', /Roses are red/],
['testFile2', /Violets are orange/]
];
assert.throws(yoAssert.noFileContent.bind(yoAssert, arg));
assert.throws(yoAssert.noFileContent.bind(yoAssert, arg), /.*AssertionError.*/);
});
});

Expand All @@ -158,8 +158,8 @@ describe('yeoman-assert', () => {
assert.throws(yoAssert.textEqual.bind(yoAssert,
'I have a yellow cat',
'I have a brown cat'
));
});
), /.*AssertionError.*/);
}, /.*AssertionError.*/);

it('pass with two similar simple lines with different new line types', () => {
assert.doesNotThrow(yoAssert.textEqual.bind(yoAssert,
Expand All @@ -183,14 +183,14 @@ describe('yeoman-assert', () => {
});

it('fails if methods are missing', function () {
assert.throws(yoAssert.implement.bind(yoAssert, this.subject, this.interfaceMore));
assert.throws(yoAssert.implement.bind(yoAssert, this.subject, this.interfaceMore), /.*AssertionError.*/);
});

it('allow interface to be an object (using its object.keys)', function () {
const interfacePass = {foo: noop};
const interfaceFail = {yop: noop};
assert.doesNotThrow(yoAssert.implement.bind(yoAssert, this.subject, interfacePass));
assert.throws(yoAssert.implement.bind(yoAssert, this.subject, interfaceFail));
assert.throws(yoAssert.implement.bind(yoAssert, this.subject, interfaceFail), /.*AssertionError.*/);
});

it('when object is passed in, it only check it implements the methods', function () {
Expand All @@ -210,7 +210,7 @@ describe('yeoman-assert', () => {
});

it('fails if methods are present', function () {
assert.throws(yoAssert.notImplement.bind(yoAssert, this.subject, ['foo']));
assert.throws(yoAssert.notImplement.bind(yoAssert, this.subject, ['foo']), /.*AssertionError.*/);
});
});

Expand Down Expand Up @@ -242,18 +242,39 @@ describe('yeoman-assert', () => {
}));
});

it('pass if array is missing but is expected to be empty', () => {
assert.doesNotThrow(yoAssert.objectContent.bind(yoAssert, {
b: undefined
}, {
b: []
}));
});

it('fails if object does not contain a key', () => {
assert.throws(yoAssert.objectContent.bind(yoAssert, {}, {
a: 'foo'
}));
}), /.*AssertionError.*/);
});

it('fails if nested object does not contain a key', () => {
assert.throws(yoAssert.objectContent.bind(yoAssert, {
a: {}
}, {
a: {b: 'foo'}
}));
}), /.*AssertionError.*/);
});

it('fails if deep nested object does not contain a key', () => {
assert.throws(yoAssert.objectContent.bind(yoAssert, {
}, {
a: {b: 'foo'}
}), /.*AssertionError.*/);
});
it('fails if deep nested array does not contain a item', () => {
assert.throws(yoAssert.objectContent.bind(yoAssert, {
}, {
a: [{b: 'foo'}]
}), /.*AssertionError.*/);
});
});

Expand All @@ -263,32 +284,40 @@ describe('yeoman-assert', () => {
a: 'foo'
}, {
a: 'foo'
}));
}), /.*AssertionError.*/);
});

it('pass if object contains nested objects and arrays', () => {
it('fail if object contains nested objects and arrays', () => {
assert.throws(yoAssert.noObjectContent.bind(yoAssert, {
a: {b: 'foo'},
b: [0, 'a'],
c: 'a'
}, {
a: {b: 'foo'},
b: [0, 'a']
}));
}), /.*AssertionError.*/);
});

it('pass if array is incomplete', () => {
it('fail if array is incomplete', () => {
assert.throws(yoAssert.noObjectContent.bind(yoAssert, {
b: [0, 'a']
}, {
b: [0]
}));
}), /.*AssertionError.*/);
});

it('fail if array is missing but is expected to be not empty', () => {
assert.throws(yoAssert.noObjectContent.bind(yoAssert, {
b: undefined
}, {
b: []
}), /.*AssertionError.*/);
});

it('pass if object does not contain a key', () => {
assert.doesNotThrow(yoAssert.noObjectContent.bind(yoAssert, {}, {
a: 'foo'
}));
}), /.*AssertionError.*/);
});

it('pass if nested object does not contain a key', () => {
Expand All @@ -298,6 +327,21 @@ describe('yeoman-assert', () => {
a: {b: 'foo'}
}));
});

it('pass if deep nested object does not contain a key', () => {
assert.doesNotThrow(yoAssert.noObjectContent.bind(yoAssert, {
}, {
a: {b: 'foo'}
}));
});

it('pass if deep nested array does not contain a key', () => {
assert.doesNotThrow(yoAssert.noObjectContent.bind(yoAssert, {
}, {
a: [{b: 'foo'}]
}));
});

});

describe('.jsonFileContent()', () => {
Expand All @@ -319,16 +363,16 @@ describe('yeoman-assert', () => {
assert.throws(yoAssert.jsonFileContent.bind(yoAssert, file, {
a: {b: 1},
b: 'a'
}));
}), /.*AssertionError.*/);

assert.throws(yoAssert.jsonFileContent.bind(yoAssert, file, {
a: {b: 3},
b: [1]
}));
}), /.*AssertionError.*/);
});

it('fails if file does not exists', () => {
assert.throws(yoAssert.jsonFileContent.bind(yoAssert, 'does-not-exist', {}));
assert.throws(yoAssert.jsonFileContent.bind(yoAssert, 'does-not-exist', {}), /.*no such file or directory*/);
});
});

Expand All @@ -343,7 +387,7 @@ describe('yeoman-assert', () => {
assert.throws(yoAssert.noJsonFileContent.bind(yoAssert, file, {
a: {b: 1},
b: [1, 2]
}));
}), /.*AssertionError.*/);
});

it('pass if file does not contain the keys', () => {
Expand All @@ -359,7 +403,7 @@ describe('yeoman-assert', () => {
});

it('fails if file does not exists', () => {
assert.throws(yoAssert.noJsonFileContent.bind(yoAssert, 'does-not-exist', {}));
assert.throws(yoAssert.noJsonFileContent.bind(yoAssert, 'does-not-exist', {}), /.*no such file or directory*/);
});
});
});