1

In my model definition, I have

# models/my_model.rb

# == Schema Information
#
# Table name: my_models
#
#  id               :bigint           not null, primary key
#  another_model_id :bigint
#  field_1          :string
#  field_2          :string
#  created_at       :datetime         not null
#  updated_at       :datetime         not null
#
# Indexes
#
#  index_my_models_on_another_model_id  (another_model_id) UNIQUE

class MyModel < ApplicationRecord

  belongs_to :another_model

  def update_from_api_response(api_response)
    $stderr.puts("UPDATE")
    self.field_1 = api_response[:field_1]
    self.field_2 = api_response[:field_2]
  end

  def update_my_model!(api_response)
    ApplicationRecord.transaction do
      $stderr.puts("HELLO")
      update_from_api_response(api_response)
      $stderr.puts("WORLD")
      self.save!
    end
  end
end

I put in some puts statements to check whether my code entered the function. If everything works alright, the program should log "HELLO", "UPDATE", then "WORLD".

In my model spec I have

# spec/models/my_model_spec.rb

RSpec.describe MyModel, type: :model do
  let(:my_model) { create(:my_model) }
  let(:api_response) {
    {
      :field_1 => "field_1",
      :field_2 => "field_2",
    }
  }

  describe("update_my_model") do
    it "should update db record" do
      expect(my_model).to receive(:update_from_api_response)
          .with(api_response)
      expect(my_model).to receive(:save!)
      expect{ my_model.update_my_model!(api_response) }
        .to change{ my_model.field_1 }
    end
  end
end

The factory object for MyModel is defined like this (it literally does not do anything)

# spec/factories/my_models.rb

FactoryBot.define do
  factory :my_model do

  end
end

The output from the puts (this appears before the error message)

HELLO
WORLD

Interestingly, "UPDATE" is not printed, but it passes the receive test.

The change match test fails, and the output from the console is as follows

1) MyModel update_my_model should update db record
     Failure/Error:
       expect{ my_model.update_my_model(api_response) }
        .to change{ my_model.field_1 }

       expected `my_model.field_1` to have changed, but is still nil
     # ./spec/models/my_model_spec.rb
     # ./spec/rails_helper.rb

I suspected that it might have something to do with me wrapping the update within ApplicationRecord.transaction do but removing that does nothing as well. "UPDATE" is not printed in both cases.

I've also changed the .to receive(:update_from_api_response) to .to_not receive(:updated_from_api_response) but it throws an error saying that the function was called (but why is "UPDATE" not printed then?). Is there something wrong with the way I'm updating my functions? I'm new to Ruby so this whole self syntax and whatnot is unfamiliar and counter-intuitive. I'm not sure if I "updated" my model field correctly.

Thanks!

Link to Git repo: https://github.com/jzheng13/rails-tutorial.git

absolutelydevastated
  • 1,657
  • 1
  • 11
  • 28

2 Answers2

2

When you call expect(my_model).to receive(:update_from_api_response).with(api_response) it actually overrides the original method and does not call it.

You can call expect(my_model).to receive(:update_from_api_response).with(api_response).and_call_original if you want your original method to be called too.

Anyway, using "expect to_receive" and "and_call_original" rings some bells for me, it means you are testing two different methods in one test and the tests actually depends on implementation details instead of an input and an output. I would run two different tests: test that "update_from_api_response" changes the fields you want, and maybe test that "update_my_model!" calls "update_from_api_response" and "save!" (no need to test the field change, since that would be covered on the "update_from_api_response" test).

arieljuod
  • 15,460
  • 2
  • 25
  • 36
  • I'm not testing `update_from_api_response`. The function I'm testing is `update_my_model` which is supposed to call `update_from_api_response`. – absolutelydevastated Sep 15 '19 at 14:32
  • But the thing is that my actual test tests another method which calls `update_my_model` and then `update_from_api_response`. I also use `receive` to test that `update_from_api_response` is called and the logged output is the same ("HELLO", "WORLD"). So even though `receive` supposedly "replaced the method, it only did so for `update_from_api_response` instead of `update_from_model`. – absolutelydevastated Sep 15 '19 at 14:37
  • I know, that's exactly my point: if you don't want to test `update_from_api_response` on that test, then there's no point on wanting the original code to be called and checking the `field_1` value changed. `update_from_api_response` is responsible for that change, if you don't want to test that method then don't test that change since the method you are actually testing is not responsible of changing that value. – arieljuod Sep 15 '19 at 14:39
  • What I'm trying to say is that you are testing more things that you should on a single test. You can split the test in smaller tests easier to tests each and you'll have the same coverage with cleaner and more decoupled tests. – arieljuod Sep 15 '19 at 14:41
  • Yes, but that doesn't really explain the behaviour of the function spec. The exact same thing happens even when I'm testing a function that's one level higher, and `update_my_model` prints "HELLO" and "WORLD" but doesn't print "UPDATE". If that is so, it cannot be that `receive` overrides the function. – absolutelydevastated Sep 15 '19 at 14:51
  • Can you show that higher level test? `expect to receive` does override the method, that's why you have the `and_call_original` method. I guess your higher level test is not exactly the same as the one you have on the question, do you have `expect(...).to receive(:update_my_model!)` on that test? – arieljuod Sep 15 '19 at 15:12
  • Yes I do. The higher level function calls the API and returns the JSON results, with a `rescue` that catches the possible errors that might arise. It also uses `receive` for `update_from_api_response` which is called within the function body. – absolutelydevastated Sep 16 '19 at 01:04
  • I insist, show that test you are talking about since you say it's not the same on the question and you seem to not like the answer but you are not being clear on what does your test really do (you try to explain it, but the actual code will be clear enough). – arieljuod Sep 16 '19 at 03:35
1

Thank you, the separate Github file works wonders.

This part works fine:

Put it in a separate expectation and it works fine:

describe("update_my_model") do it "should update db record" do # This works expect{ my_model.update_my_model!(api_response) }.to change{ my_model.field_one } end end

How is it triggered?

But here is your problem:

expect(my_model).to receive(:update_from_api_response).with(api_response) expect(my_model).to receive(:save!)

This means that you are expecting my model to have update_from_api_response to be called with the api_response parameter passed in. But what is triggering that? Of course it will fail. I am expecting my engine to start. But unless i take out my car keys, and turn on the ignition, it won't start. But if you are expecting the car engine to start without doing anything at all - then of course it will fail! Please refer to what @arieljuod has mentioned above.

Also why do you have two methods: update_from_api_response and update_my_model! which both do the same thing - you only need one?

BenKoshy
  • 33,477
  • 14
  • 111
  • 80
  • 1
    `update_from_api_response` and `update_my_model!` does two different things they don't do the same thing. One calls the other and saves de record inside a transaction. It's a design decision I guess, maybe he calls `update_from_api_response` somewhere else too. – arieljuod Sep 16 '19 at 03:34