I feel like the value of using “to satisfy” with an empty object is pretty low.
perhaps changing that would be acceptable. if you write
expect({foo: ‘bar’}, ‘to satisfy’, {}) you should use ’to be an object’ instead or something.
This super naive hack only breaks 11 tests in core, mostly in error messages where
plainObject shows up instead of object, and some stuff with the obscure arguments type:diff --git a/lib/assertions.js b/lib/assertions.js
index 47905ce9..14850862 100644
--- a/lib/assertions.js
+++ b/lib/assertions.js
@@ -1437,7 +1437,7 @@ module.exports = (expect) => {
);
expect.addAssertion(
- '<Error> to [exhaustively] satisfy <object>',
+ '<Error> to [exhaustively] satisfy <plainObject>',
(expect, subject, value) => {
const valueType = expect.argTypes[0];
const subjectKeys = expect.subjectType.getKeys(subject);
diff --git a/lib/createTopLevelExpect.js b/lib/createTopLevelExpect.js
index 6b79f498..fc76f695 100644
--- a/lib/createTopLevelExpect.js
+++ b/lib/createTopLevelExpect.js
@@ -1720,7 +1720,11 @@ expectPrototype.standardErrorMessage = function (output, options) {
if (options && options.compact) {
options.compactSubject = (output) => {
- output.jsFunctionName(this.subjectType.name);
+ let typeName = this.subjectType.name;
+ if (typeName === 'plainObject') {
+ typeName = 'object';
+ }
+ output.jsFunctionName(typeName);
};
}
return createStandardErrorMessage(
diff --git a/lib/types.js b/lib/types.js
index b00bdecb..25371251 100644
--- a/lib/types.js
+++ b/lib/types.js
@@ -897,6 +897,17 @@ module.exports = function (expect) {
},
});
+ expect.addType({
+ base: 'object',
+ name: 'plainObject',
+ identify(obj) {
+ return (
+ this.baseType.identify(obj) &&
+ Object.getPrototypeOf(obj) === Object.prototype
+ );
+ },
+ });
+
expect.addType({
base: 'object',
name: 'function',
Let's see what the others think. I'm on my way to bed, talk to you later :)
tty, good night!
Simpler and less invasive fix:
diff --git a/lib/assertions.js b/lib/assertions.js
index 47905ce9..63bef536 100644
--- a/lib/assertions.js
+++ b/lib/assertions.js
@@ -1949,6 +1949,9 @@ module.exports = (expect) => {
if (valueType.is('array-like') && !subjectIsArrayLike) {
expect.fail();
}
+ if (Object.getPrototypeOf(value) !== Object.prototype) {
+ expect.fail();
+ }
const subjectKeys = subjectType.getKeys(subject);
const valueKeys = valueType.getKeys(value);That doesn't make any current tests fail. If you want <object> to satisfy <some more specific type> you should probably create a more specific assertion anyway.
@boneskull, so that function that now returns a
Set was used to retrieve the RHS of the to satisfy assertion in the test suite? I'm usually very conservative with too much complexity in tests, preferring to always assert against constants. But people have different styles :)
There's 5 failing u-messy tests, but everything else works: https://github.com/unexpectedjs/unexpected/pull/781/checks?check_run_id=1541877571
re unexpectedjs/unexpected#781 - that handles one case, but just noting that we still have the issue the other way round. Namely, if you refactored some code and changed the subject from an
{} to a new Map(), you will have weird results because the properties attached to the Map object will be checked rather than the .set() mappings it contains
perhaps that's a vote for having Map in core so that just can't happen - to answer your Q @papandreou, I think the plugin is actually in a good state as it stands - I did a pretty extensive rework leading up to v2 that ported all the v11 "to satisfy" behaviours
It's a super inefficient implementation for now, but LMK if there's appetite for getting something like this in, or if you have feedback about the syntax or feature set.
I hope we'd be able to use the type generation part of unassessed on top :)
@papandreou sorry for not looking at your camel case spike yet, I'm in Svendborg until Wednesday, but I'll take a look when I have some time.
No rush at all! There's still plenty of stuff to fix on it. It would be nice to get some feedback on the api when you have time, you can just ignore everything except the new test: https://github.com/unexpectedjs/unexpected/pull/784/files#diff-d03ea72142f3971d3a3e4ff1b79deedba7269b7530be66d24150ab4f20520473R1
By default mocha looks for test files in
./test/*.js, and with the --recursive option it will use the equivalent of ./test/**/*.js. I'd like to change that to **/test/*.js and ideally through configuration. I cannot find anything about it in the docs nor on the website. Do you have any clue about how to do that?
Config files in jason/yaml, async functions for tests instead of done callbacks etc