5 ms·

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 ho

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?

Dirlewanger 7y ago
I'd start with removing that `update` out of the `before_filter`; any persistence calls should be only happening in the main controller action.
- gingerlime 7y ago
  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.
DougBTX 7y ago
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.
- snicky 7y ago
  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.
- gingerlime 7y ago
  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...
Tainnor 7y ago
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"

Dirlewanger 7y ago

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

gingerlime 7y ago
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.

DougBTX 7y ago

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.

snicky 7y ago
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.
gingerlime 7y ago
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...

Tainnor 7y ago

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"