Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Make spy formatter `%C` have empty lines #203

Merged
merged 2 commits into from

2 participants

@alFReD-NSH

This really makes life easier.

@cjohansen
Owner

Do you have a quick visual diff? E.g. an example error before and after?

@alFReD-NSH

Thought I should do this. But was too lazy :P
This is exactly the case that made me need this.

Before:

AssertError: expected spy to be called once but was called twice
    spy({
  arr1: [Saturday October 20, 2012 9:00:00am..Saturday October 20, 2012 12:00:00pm, Monday October 22, 2012 9:00:00am..Monday October 22, 2012 12:00:00pm],
  arr2: [],
  user: "user1"
}, Sunday October 21, 2012 12:00:00am..Monday October 21, 2013 12:00:00am) => [{ fromType: "pilot-report", violation: true }, { fromType: "planner", violation: true }]
    spy({
  arr1: [Saturday October 20, 2012 9:00:00am..Saturday October 20, 2012 12:00:00pm, Monday October 22, 2012 9:00:00am..Monday October 22, 2012 12:00:00pm],
  arr2: [],
  user: "user1"
}, Sunday October 21, 2012 12:00:00am..Monday October 21, 2013 12:00:00am) => [{ fromType: "planner", violation: true }]

After:

AssertError: expected spy to be called once but was called twice

    spy({
  arr1: [Saturday October 20, 2012 9:00:00am..Saturday October 20, 2012 12:00:00pm, Monday October 22, 2012 9:00:00am..Monday October 22, 2012 12:00:00pm],
  arr2: [],
  user: "user1"
}, Sunday October 21, 2012 12:00:00am..Monday October 21, 2013 12:00:00am) => [{ fromType: "pilot-report", violation: true }, { fromType: "planner", violation: true }]

    spy({
  arr1: [Saturday October 20, 2012 9:00:00am..Saturday October 20, 2012 12:00:00pm, Monday October 22, 2012 9:00:00am..Monday October 22, 2012 12:00:00pm],
  arr2: [],
  user: "user1"
}, Sunday October 21, 2012 12:00:00am..Monday October 21, 2013 12:00:00am) => [{ fromType: "planner", violation: true }]
@cjohansen
Owner

Right. In this case it makes sense. However, for shorter examples this would be a regression. My suggestion is to only inject an additional line break if the individual spy spans more than one line. What do you think?

@alFReD-NSH

That make it a bit complex, but that works. We can make it if each call is more than one line, then they add an empty line above them.

@cjohansen
Owner

Sure that works. My main issue is that I don't want too fluffy output for cases where there's a few calls and relatively few/short arguments. In my experience, that is the most common case.

@alFReD-NSH

Though I'm not sure how to express the test case. There's no test case specifically for the formatting. Should I create? Or just add to the assertion test cases(seems wrong but easiest.)

@alFReD-NSH

On the second thoughts, I just write a set of test cases for spy.printf.

@alFReD-NSH

@cjohansen Now, they will add new lines if the call before them was more than one line.

@cjohansen cjohansen merged commit 299f208 into cjohansen:master
@cjohansen
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 91 additions and 1 deletion.
  1. +5 −1 lib/sinon/spy.js
  2. +86 −0 test/sinon/spy_test.js
View
6 lib/sinon/spy.js
@@ -340,7 +340,11 @@
var calls = [];
for (var i = 0, l = spy.callCount; i < l; ++i) {
- push.call(calls, " " + spy.getCall(i).toString());
+ var stringifiedCall = " " + spy.getCall(i).toString();
+ if (/\n/.test(calls[i - 1])) {
+ stringifiedCall = "\n" + stringifiedCall;
+ }
+ push.call(calls, stringifiedCall);
}
return calls.length > 0 ? "\n" + calls.join("\n") : "";
View
86 test/sinon/spy_test.js
@@ -2635,6 +2635,92 @@ if (typeof require === "function" && typeof module === "object") {
assert(argSpy2.getCall(0).threw("2"));
assert(argSpy2.getCall(1).threw("3"));
}
+ },
+ "printf" : {
+ "name" : {
+ "named" : function () {
+ var named = sinon.spy(function cool() { });
+ assert.equals(named.printf('%n'), 'cool');
+ },
+ "anon" : function () {
+ var anon = sinon.spy(function () {});
+ assert.equals(anon.printf('%n'), 'spy');
+
+ var noFn = sinon.spy();
+ assert.equals(noFn.printf('%n'), 'spy');
+ },
+ },
+ "count" : function () {
+ // Throwing just to make sure it has no effect.
+ var spy = sinon.spy(sinon.stub().throws());
+ function call() {
+ try {
+ spy();
+ } catch (e) {}
+ }
+
+ call();
+ assert.equals(spy.printf('%c'), 'once');
+ call();
+ assert.equals(spy.printf('%c'), 'twice');
+ call();
+ assert.equals(spy.printf('%c'), 'thrice');
+ call();
+ assert.equals(spy.printf('%c'), '4 times');
+ },
+ "calls" : {
+ oneLine : function () {
+ function test(arg, expected) {
+ var spy = sinon.spy();
+ spy(arg);
+ assert.equals(spy.printf('%C'), '\n ' + expected);
+ }
+
+ test(true, 'spy(true)');
+ test(false, 'spy(false)');
+ test(undefined, 'spy(undefined)');
+ test(1, 'spy(1)');
+ test(0, 'spy(0)');
+ test(-1, 'spy(-1)');
+ test(-1.1, 'spy(-1.1)');
+ test(Infinity, 'spy(Infinity)');
+ test(['a'], 'spy(["a"])');
+ test({ a : 'a' }, 'spy({ a: "a" })');
+ },
+ multiline : function () {
+ var str = 'spy\ntest';
+ var spy = sinon.spy();
+
+ spy(str);
+ spy(str);
+ spy(str);
+
+ assert.equals(spy.printf('%C'),
+ '\n spy(' + str + ')' +
+ '\n\n spy(' + str + ')' +
+ '\n\n spy(' + str + ')');
+
+ spy.reset();
+
+ spy('test');
+ spy('spy\ntest');
+ spy('spy\ntest');
+
+ assert.equals(spy.printf('%C'),
+ '\n spy(test)' +
+ '\n spy(' + str + ')' +
+ '\n\n spy(' + str + ')');
+ }
+ },
+ "thisValues" : function () {
+ var spy = sinon.spy();
+ spy();
+ assert.equals(spy.printf('%t'), 'undefined');
+
+ spy.reset();
+ spy.call(true);
+ assert.equals(spy.printf('%t'), 'true');
+ }
}
});
}());
Something went wrong with that request. Please try again.