Skip to content

Commit

Permalink
Use util.inspect for formatting
Browse files Browse the repository at this point in the history
As part of a larger effort, we are retiring
`@sinonjs/formatio`. When it was created Node was
quite immature and there were no other community
projects for solid object formatting.

Ten years later, Node is mature and has great
support for formatting all sorts of values.

Removing the use of `@sinonjs/formatio` in favour
of Node's `util.inspect` removes an entire
project's worth of maintenance burden from the
organisation.

In practial terms, it means that some output from
`sinon` will look slightly different. This won't
cause a new major release as this output is only
intended to be read by humans and not be part of
an API promise.
  • Loading branch information
mroderick committed Jan 6, 2021
1 parent 2608a71 commit 6b3dac6
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 81 deletions.
10 changes: 2 additions & 8 deletions lib/sinon/util/core/format.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
"use strict";

var formatio = require("@sinonjs/formatio");

var formatter = formatio.configure({
quoteStrings: false,
limitChildrenCount: 250
});

var inspect = require("util").inspect;
var customFormatter;

function format() {
if (customFormatter) {
return customFormatter.apply(null, arguments);
}

return formatter.ascii.apply(formatter, arguments);
return inspect.apply(inspect, arguments);
}

format.setFormatter = function(aCustomFormatter) {
Expand Down
108 changes: 74 additions & 34 deletions test/assert-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ var sinonAssert = require("../lib/sinon/assert");
var match = require("@sinonjs/samsam").createMatcher;
var assert = referee.assert;
var refute = referee.refute;
var inspect = require("util").inspect;

function requiresValidFake(method) {
it("should fail with non-function fake", function() {
Expand Down Expand Up @@ -1596,7 +1597,12 @@ describe("assert", function() {

assert.equals(
this.message("calledOn", this.obj.doSomething, this.obj),
"expected doSomething to be called with [Oh yeah] as this but was called with [Oh no], [Oh well]"
"expected doSomething to be called with " +
inspect(this.obj) +
" as this but was called with " +
inspect(obj) +
", " +
inspect(obj2)
);
});

Expand All @@ -1622,8 +1628,14 @@ describe("assert", function() {

assert.equals(
this.message("alwaysCalledOn", this.obj.doSomething, this.obj),
"expected doSomething to always be called with [Oh yeah] as this but was called with " +
"[Oh no], [Oh well], [Oh yeah]"
"expected doSomething to always be called with " +
inspect(this.obj) +
" as this but was called with " +
inspect(obj) +
", " +
inspect(obj2) +
", " +
inspect(this.obj)
);
});

Expand Down Expand Up @@ -1657,7 +1669,7 @@ describe("assert", function() {
color.green("1") +
" \n" +
"3\n" +
'"hey"'
inspect('"hey"')
);
});

Expand All @@ -1674,13 +1686,14 @@ describe("assert", function() {
color.green("1") +
" \n" +
"3\n" +
'"hey"\n' +
inspect('"hey"') +
"\n" +
"Call 2:\n" +
"1\n" +
"3\n" +
color.red('"not"') +
color.red(inspect('"not"')) +
" " +
color.green('"hey"') +
color.green(inspect('"hey"')) +
" "
);
});
Expand All @@ -1706,18 +1719,45 @@ describe("assert", function() {
},
"fifth"
];
assert.equals(
this.message("calledWith", this.obj.doSomething, expectedArg).replace(/ at.*/g, ""),
"expected doSomething to be called with arguments \n" +
"[{\n" +
' first: "a",\n' +
color.red(" mismatchKey: true,\n") +
color.green(" mismatchKeyX: true,\n") +
" second: { nest: true },\n" +
color.red(" third: [{ fourth: { nest: true } }]\n") +
color.green(" third: [{ fourth: { nest: false } }]\n") +
'}, "fifth"]'
);

var actual = this.message("calledWith", this.obj.doSomething, expectedArg);

/**
* Unfortunately, `util.inspect` behaves differently in node than
* it does in browsers, so we need to detect the difference in order
* to set the correct value for expected `expected`.
*
* In node the output uses more whitespace than in browsers.
*
* @type {Boolean}
*/
var usesCondensedFormat =
inspect([
{ apple: "e4d13f88-9b9b-4e05-8abb-f76df2d4ef40" },
{ pear: "841b661f-80f4-4560-9cf4-133dcffd240c" }
]).indexOf("[ {") === 0;

var expected = usesCondensedFormat
? "expected doSomething to be called with arguments \n" +
"[ { first: 'a',\n" +
" second: { nest: true },\n" +
" third: [ [Object] ],\n" +
color.red(" mismatchKey: true },\n") +
color.green(" mismatchKeyX: true },\n") +
" 'fifth' ]"
: "expected doSomething to be called with arguments \n" +
"[\n" +
" {\n" +
" first: 'a',\n" +
" second: { nest: true },\n" +
" third: [ [Object] ],\n" +
color.red(" mismatchKey: true\n") +
color.green(" mismatchKeyX: true\n") +
" },\n" +
" 'fifth'\n" +
"]";

assert.equals(actual, expected);
});

it("assert.calledWith exception message with a missing argument", function() {
Expand Down Expand Up @@ -1892,7 +1932,7 @@ describe("assert", function() {
color.green("4") +
" \n" +
"3\n" +
'"hey"'
inspect('"hey"')
);
});

Expand All @@ -1907,13 +1947,13 @@ describe("assert", function() {
"1\n" +
color.red("3") +
" " +
color.green('"hey"') +
color.green(inspect('"hey"')) +
" \n" +
color.red('"hey"') +
color.red(inspect('"hey"')) +
"\n" +
"Call 2:\n" +
"1\n" +
'"hey"'
inspect('"hey"')
);
});

Expand All @@ -1928,13 +1968,13 @@ describe("assert", function() {
"1\n" +
color.red("3") +
" " +
color.green('"hey"') +
color.green(inspect('"hey"')) +
" \n" +
color.red('"hey"') +
color.red(inspect('"hey"')) +
"\n" +
"Call 2:\n" +
"1\n" +
'"hey"'
inspect('"hey"')
);
});

Expand All @@ -1943,7 +1983,7 @@ describe("assert", function() {

assert.equals(
this.message("calledWithExactly", this.obj.doSomething, 1, 3).replace(/ at.*/g, ""),
"expected doSomething to be called with exact arguments \n1\n3\n" + color.red('"hey"')
"expected doSomething to be called with exact arguments \n1\n3\n" + color.red(inspect('"hey"'))
);
});

Expand All @@ -1962,7 +2002,7 @@ describe("assert", function() {
color.green("1") +
" \n" +
"3\n" +
'"bob"'
inspect('"bob"')
);

this.obj.doSomething();
Expand All @@ -1974,7 +2014,7 @@ describe("assert", function() {
"\n" +
color.red("3") +
"\n" +
color.red(JSON.stringify('"bob"')) +
color.red(inspect(JSON.stringify('"bob"'))) +
"\n" +
"Call 2:"
);
Expand All @@ -1988,7 +2028,7 @@ describe("assert", function() {
"expected doSomething to be called with arguments \n" +
color.red(1234) +
" " +
color.green('"1234"') +
color.green(inspect('"1234"')) +
" "
);
});
Expand All @@ -2003,7 +2043,7 @@ describe("assert", function() {
"Call 1:\n" +
"1\n" +
"3\n" +
color.red('"hey"') +
color.red(inspect('"hey"')) +
"\n" +
"Call 2:\n" +
"1\n" +
Expand Down Expand Up @@ -2035,7 +2075,7 @@ describe("assert", function() {

assert.equals(
this.message("threw", this.obj.doSomething).replace(/ at.*/g, ""),
"doSomething did not throw exception\n doSomething(1, 3, hey)\n doSomething(1, 3)"
"doSomething did not throw exception\n doSomething(1, 3, 'hey')\n doSomething(1, 3)"
);
});

Expand All @@ -2045,14 +2085,14 @@ describe("assert", function() {

assert.equals(
this.message("alwaysThrew", this.obj.doSomething).replace(/ at.*/g, ""),
"doSomething did not always throw exception\n doSomething(1, 3, hey)\n doSomething(1, 3)"
"doSomething did not always throw exception\n doSomething(1, 3, 'hey')\n doSomething(1, 3)"
);
});

it("assert.match exception message", function() {
assert.equals(
this.message("match", { foo: 1 }, [1, 3]),
"expected value to match\n expected = [1, 3]\n actual = { foo: 1 }"
"expected value to match\n expected = [ 1, 3 ]\n actual = { foo: 1 }"
);
});
});
Expand Down
20 changes: 10 additions & 10 deletions test/mock-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ describe("sinonMock", function() {
expectation.verify();
},
{
message: "Expected myMeth([...]) once (never called)"
message: "Expected myMeth('[...]') once (never called)"
}
);
});
Expand Down Expand Up @@ -884,8 +884,8 @@ describe("sinonMock", function() {
},
{
message:
"Expected method([...]) thrice (never called)\n" +
"Expected method(42[, ...]) once (never called)"
"Expected method('[...]') thrice (never called)\n" +
"Expected method(42, '[...]') once (never called)"
}
);
});
Expand Down Expand Up @@ -960,7 +960,7 @@ describe("sinonMock", function() {
{
message:
"Unexpected call: method()\n" +
" Expectation met: method(1[, ...]) once\n" +
" Expectation met: method(1, '[...]') once\n" +
" Expected method(42) thrice (never called)"
}
);
Expand All @@ -982,7 +982,7 @@ describe("sinonMock", function() {
mock.verify();
},
{
message: "Expected method(42) thrice (never called)\nExpectation met: method(1[, ...]) once"
message: "Expected method(42) thrice (never called)\nExpectation met: method(1, '[...]') once"
}
);
});
Expand All @@ -996,7 +996,7 @@ describe("sinonMock", function() {
mock.verify();
},
{
message: "Expected method([...]) at least once (never called)"
message: "Expected method('[...]') at least once (never called)"
}
);
});
Expand All @@ -1014,7 +1014,7 @@ describe("sinonMock", function() {
object.method();
},
{
message: "Unexpected call: method()\n Expectation met: method([...]) at most twice"
message: "Unexpected call: method()\n Expectation met: method('[...]') at most twice"
}
);
});
Expand All @@ -1037,8 +1037,8 @@ describe("sinonMock", function() {
{
message:
"Unexpected call: method(2)\n" +
" Expectation met: method([...]) at least once\n" +
" Expectation met: method(2[, ...]) once"
" Expectation met: method('[...]') at least once\n" +
" Expectation met: method(2, '[...]') once"
}
);
});
Expand All @@ -1054,7 +1054,7 @@ describe("sinonMock", function() {
mock.verify();
},
{
message: "Expected method([...]) at least once and at most twice (never called)"
message: "Expected method('[...]') at least once and at most twice (never called)"
}
);
});
Expand Down
6 changes: 3 additions & 3 deletions test/proxy-call-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ describe("sinonSpy.call", function() {
.getCall(0)
.toString()
.replace(/ at.*/g, ""),
"doIt(42, Hey)"
"doIt(42, 'Hey')"
);
});

Expand All @@ -1092,7 +1092,7 @@ describe("sinonSpy.call", function() {
.getCall(0)
.toString()
.replace(/ at.*/g, ""),
"doIt(42, Hey) => 42"
"doIt(42, 'Hey') => 42"
);
});

Expand All @@ -1105,7 +1105,7 @@ describe("sinonSpy.call", function() {
.getCall(0)
.toString()
.replace(/ at.*/g, ""),
"doIt(42, Hey) => (empty string)"
"doIt(42, 'Hey') => ''"
);
});

Expand Down
Loading

0 comments on commit 6b3dac6

Please sign in to comment.