Expand .with message to include actual results

[robcolburn]

I ❤️ chai-spies especially with, but I found the output left me wanting a bit more. For a long while, I would just write in some hacky stuff manually. But, enough is enough, it's time to do something about it.

I dug into #{act}, like #29 did. It's cool API, you just pass in another argument like calls to this.assert to get it working. But, I couldn't get it do anything helpful info for me. My hack still produced better data because I usually have somewhat deep object I need to validate (I really need something like chai's equal with the whole +/- thing). So, I did the next best thing I could. I found chai has (a wrapper for Node's inspect) which it uses a lot internally, and I brought into our message. This gets a little chatty, especially since my messages are more wordy, but I find I want more data when these things go wrong.

So, what does it look like?

AssertionError: expected { Spy } to have been called at least once with [ { type: 'SET',
    key:
     { isAuthenticated: false,
       isAuthenticationChecked: true,
       authnError: null,
       mvpd: null },
    value: undefined } ], but it was actually not called

AssertionError: expected { Spy, 2 calls } to have been called at least once with [ { type: 'SET',
    key:
     { isAuthenticationChecked: false,
       isAuthenticated: false,
       authToken: false,
       mvpd: null,
       video: [Object] } } ], but it was actually called 2 times with:
call #1: [ { type: 'SET',
    key: 'video',
    value:
     { id: '5f53f48a-2465-4750-babe-67ee7f3657f5',
       show: [Object],
       authzError: 'User Not Authenticated Error' } } ]
call #2: [ { type: 'SET',
    key:
     { isAuthenticationChecked: false,
       isAuthenticated: false,
       authToken: false,
       mvpd: null,
       video: [Object] },
    value: undefined } ]

Aside: I'm spying on Redux dispatch function. If you know Redux, yes, we have some anti-pattern in our architecture, we're dealing with it.

[meeber]

@robcolburn Thank you for the PR and a thousand apologies for the delay :D

I like where you're going with this, and I agree with your conclusion that #{act} isn't the best approach in this case; the nested arrays it produces is messy. However, I'd like to suggest that we use _.objDisplay instead of _.inspect.

_.objDisplay will respect a consumer's chai.config.truncateThreshold setting, allowing them to control how chatty their output is allowed to be.

Also, I'm not sure how I feel about the lack of indentation on the newlines for each call. I think I could go either way, but I feel myself leaning toward wanting a 2 or 4 space indentation.

What are your thoughts? @keithamus? @imjacobclark?

(Note: We'd definitely want the final solution to include tests as well!)

[imjacobclark]

Personally, I think this is excellent, anything that outputs what actually happened to track down why the test failed is a huge +1.

[robcolburn]

@meeber,

1. `_.objDisplay. Haven't seen that before, that sounds helpful. I'll try that out on Mon.
2. Indentation. I could add some, though I'm guessing I'll need to parse the output of inspect/objDisplay and add indentation there as well. IMO my output gets quite large without adding space.
3. Tests. Sure, I did some fiddling with the output to match to match the currentwith tests. What would you like to see out of these new test cases?

[meeber]

@robcolburn I'm still not sure how I feel about indentation vs no-indentation. But if indentation were to be added, I think it might be as easy as changing:

return '\ncall #' + (i + 1)

to

return '\n    call #' + (i + 1)

Ideally, there should be a test added for each new message format that you've added, verifying the actual .message of each assertion error, either with .match or .equal.

[robcolburn]

@meeber,
What do you think this example should look like?

AssertionError: expected { Spy, 2 calls } to have been called at least once with [ { type: 'SET',
    key:
     { isAuthenticationChecked: false,
       isAuthenticated: false,
       authToken: false,
       mvpd: null,
       video: [Object] } } ], but it was actually called 2 times with:
call #1: [ { type: 'SET',
    key: 'video',
    value:
     { id: '5f53f48a-2465-4750-babe-67ee7f3657f5',
       show: [Object],
       authzError: 'User Not Authenticated Error' } } ]
call #2: [ { type: 'SET',
    key:
     { isAuthenticationChecked: false,
       isAuthenticated: false,
       authToken: false,
       mvpd: null,
       video: [Object] },
    value: undefined } ]

[meeber]

@robcolburn I think that looks fine without any indentation on the "call".

[keithamus]

@robcolburn hey - have you had any time to look into this recently? I think it'd be great to get this updated and merged in!

[stalniy]

@keithamus I think it would be more preferable to enable such behavior via configuration option. Something like verbose: true. So,

1. Is there a way to read chai configuration inside chai-spies module?
2. Is there an existing option like this in chai?

[keithamus]

@stalniy I disagree. I much prefer plugins to have zero configuration. It is too high a burden on the user to curate a configuration to their needs; overall we are trying to make chai (and plugins) more usable and less configuration based.

The messages here could use some work, but they are a good start to making chai spies much more usable.