Hacker News Reader

Comment by gingerlime | Hacker News Reader

2018 ?

I share similar doubts to other commenters.

One semi-positive side effect of moving this into a class however, was testing (kinda?).

I'm never sure how to test certain bits of my code that might do something as simple as `Model.where(:x => "y").where(:y => "z").limit(3)` ... usually ending up with a bunch of `expect(...).to receive_message_chain` which feels extremely clunky (and doesn't even test all where params).

The other day, I even struggled to test an even simpler `User.update(:x => "y")`, because there was another `update` in a before filter on ApplicationController... and it easily becomes a mock-the-world situation.

Am I doing it all wrong?

View on HN

I like to think about what a test of `Model.where(:x => "y").where(:y => "z").limit(3)` is supposed to be testing. If the test is that the correct query gets generated, then a simple check to ensure that the `where` calls have been made is fine. If the test is that the query returns the correct data, then either a real database or something lighter weight but which handles queries in the same way needs to be used, since it is really a test of how the query is interpreted rather than what the query is.

Exactly. The usual way to deal with this would be to move the `where(:x => "y").where(:y => "z")` logic to a scope on the Model and test the scope in a unit test by checking if it returns the records as intended.

I’m testing the message. So that the (in this case) `.where` was called and with the right parameters... I trust ActiveRecord with the rest, and I don’t want to hit the DB.

I can’t simply use `expect(...).to receive(:where)`... even if I only cared that some `.where` was called...

I'd start with removing that `update` out of the `before_filter`; any persistence calls should be only happening in the main controller action.

But what if this is something shared between all controllers? (In our case potentially updating the locale of the user).

EDIT: although I admit it's not ideal, so probably a "smell" to consider doing differently.

don't mock what you don't own, IMHO

the classic approach is to use your own domain abstractions, mock those in unit tests, test the domain abstractions e.g. with an in-memory DB, and have some integration tests verify that everything is plugged together correctly

making assertions about your own, domain-inspired APIs makes your tests less brittle and more readable than using "receive_message_chain"

Reply on news.ycombinator.com