I’m wondering if in the Extract Class video we should be testing the implementation of recipients method, which is also a private method @benorenstein, @jferris ?
According to Sandi’s recommendations, you should expect to send outgoing messages - so I was thinking you would have a simple expectation such as
expect(parser).to have_received(:recipients)
in order to test the behavior or implementation rather than state, which is handled over in the Parser class directly? Is it enough to mock the implementation by creating a double and stubbing the recipients method, knowing that it will get called when exercising the send method?
I’m just curious regarding this point, because I have the same issue with the exercise - where I’m collaborating with another object in a private method. I’m a bit confused since I’m not testing private methods, but feel like I should be testing these types of outgoing messages to check they’re being sent?
@ralphwintle In cases where you’re stubbing out a query method - that is, a method that returns a value but doesn’t have any side effects - I find it’s best not to explicitly check that it was called.
Since there’s no reasonable way the system under test (Sender) could have produced the correct outgoing messages to Mailer without calling parser.recipients, setting a mock expectation or using a test spy would be redundant and make the test more brittle.
@jferris Ah. I think my confusion was due to my interpretation of outgoing command messages. I thought the parsing constituted a change/side effect since it returned something different, but after watching Sandi’s talk again it’s clear this is simply a query method (as you’ve stated).
I think what cleared it up for me was when she says (note I’m paraphrasing) “the result of an outgoing message being sent is that something new is created, and that we must test that message is sent, since other objects will depend upon it’s side effects”.
I suppose the parsing in the example is more akin to a ‘calculation’, and no other objects appear to be dependent on the result of calling parser.recipients.
Further, the parser.recipients code is in a private recipients method, which would characterize it as ‘sent to self’, which means it should be ignored in our tests (as per Sandi’s recommendations).
Sorry for the long-winded reply. I just wanted to write my thoughts out, perhaps someone else will find it useful reading this or have some questions too If I’m wrong in any way please let me know!
For the exercise, we are asked to create a class for removing spaces and splitting on the delimiter for email addresses. My question is should this new class ( I called mine EmailSanitizer) be added to Survey Inviter’s initialize method where @recipients is set? If it’s added there, we would lose the initial reference to @recipients and instead have a sanitized version. Just wondering what some best practices around this would be.
Thanks
If I’m wrong in any way please let me know!