How to get your butt bitten by Rails' try and rspec's allow(x).to receive(:bla)

try-and-allow.md

Why Rails' try() and rspec's allow(x).to receive(:bla) can bite you in the butt:

When actions are taken by superusers, we're supposed to append admin_ to the SystemLog event names. For example: when one of us logs in, there should be a SystemLog with admin_log_in event. When normal users log in, there's a SystemLog with log_in event.

If you check your SystemLog table, you'll see that's not currently true ;) And it hasn't been for quite a while. This is surprising, since all of our SystemLog specs are passing!

Digging deeper, we find self.prefixed_event_name(event_name) in SystemLog, which is supposed to add the admin_ prefix. That method checks if the current_user is an admin with: current_user.try(:admin?). current_user could be nil, which is why we use try here. But guess what: the method User#admin? does not exist! We recently renamed admins to superusers, so the correct call should be current_user.try(:superuser?).

But then, how can our tests be passing??

In system_log_spec.rb we find the culprit: allow(user).to receive(:admin?).and_return(true). That innocent-looking line creates the admin? method on user, so then the prefixed_event_name method correctly adds the admin_ prefix... but ONLY in our tests!

Lessons learned:

1. Use try! instead of try. try! will actually call the method even if it doesn't exist. We would have caught this error much earlier, since we would have seen NoMethodError: undefined method admin? for #<User:0x00000003b74a78> all over the place.
2. Don't use allow(x).to receive(:bla) if you don't need it. In this case, making the user an actual superuser with user.superuser! is much shorter and clearer. Also, using allow(x).to receive(:bla) tests the implementation, not the behavior.