How do you test service classes in Rails?

Hi all, I asked this on StackOverflow and got some good answers but I’m curious as to how thoughtbotters test service classes. Let’s say a User signs up. A user is created in the database, is added to the email list, and other stuff happens. How do you test this?

class UserRegistrar
  def sign_up(user)
    User.create(user) # or something to this effect
    EmailMarketing.add_to_email_list(user)
    SuperSecretClass.do_secret_stuff(user)
    LoggingThing.new.log_stuff_about(user)
  end
end

(Controller action)

def create
    UserRegistrar.sign_up(params)
    # stuff for the strong params, etc...
end

Would you

  • Test that certain methods are called? (EmailMarketing.should_receive(:add_to_email_list).with...) Then you can test the individual effects in the class that owns it.
  • Test the effect of the code itself (expect { UserRegistrar.sign_up(user) }.to change{ user.persisted? }.from(false).to(true)), which is more thorough but takes a bit longer.
  • Something else/case to case basis?

I prefer to test my classes in isolation, so I’d go with the approach you described in your first bullet.

However, rather than referencing specific class names and stubbing class objects, I’d instead inject the dependencies like this:

class UserRegistrar
  def initialize(user_attributes, email_list, secret_thing, logging_thing)
    @user_attributes = user_attributes
    @email_list = email_list
    @secret_thing = secret_thing
    @logging_thing = logging_thing
  end

  def sign_up
    user = create_user
    @email_list.add(user)
    @secret_thing.do_secret_stuff(user)
    @logging_thing.new.log_stuff_about(user)
  end

  private

  def create_user
    User.create(user_attributes)
  end
end

Then, in my tests, I’d pass in doubles and assert that they received the right messages.

4 Likes

Thanks, Ben. Would it also be better to full-POODR it or is this too excessive?

def initialize(args)
  @user_attributes = args.fetch(:user_attributes)
  @email_list = args.fetch(:email_list, DEFAULT_EMAIL_LIST)
  @secret_thing = args.fetch(:secret_thing)
  @logging_thing = args.fetch(:logging_thing, DEFAULT_LOGGER)
end
1 Like

Putting those arguments in a hash is totally fine, and probably better than having them as positional arguments.

However, I think this is a classic case of treating the symptom rather than the cause.

A class that needs 4 collaborators would almost certainly benefit from being split up. Even 3 raises my code-smell sense.

1 Like

Following on from @benorenstein’s point, possibly using some observers would split up the responsibilities of UserRegistrar. The sign_up method could simply publish a topic that says “Hey, whoever is listening, I’ve signed this guy up!”. You’d then have listeners that would do your “Secret thing” and another that would log the successful signup. You could try the new Wisper gem which looks like a good choice. Here’s a quick lightning talk about it

Thank you @benorenstein and @aaronmcadam, most probably, I’m just not structuring the app correctly. I’m not sure though how you can lessen the number of collaborators. Let’s say hypothetically, when a user signs up to myapp.com via Github, we

  1. Create a user row on the database
  2. Add user to our mailing list (MailChimp).
  3. Ask Resque/Delayed Job to queue saving the user’s avatar
  4. Save the user’s repos to the database

How would you split this class?

(I’ll check the observers, but I’m a bit tentative about them, because I end up with code in random places… if I’m just going to use a callback once I’ll usually just inline the method call.)

One of the tradeoffs of OO is indirection and not being able to see a full procedure in one place. What we get in return is more maintainable and changeable objects. This video does a great job of summing up that idea.

I also refer to the legend that is Sandi Metz on SOLID principles.

I don’t normally see reuse as the main reason to refactor.