Testing Fundamentals - Write A Feature With Tests

This topic is for the [Write A Feature With Tests] exercise in the [Testing Fundamentals] trail. Post any questions, corrections, or pointers you have to share with other Upcase subscribers.
[Write A Feature With Tests]: https://exercises.upcase.com/exercises/testing-fundamentals-write-a-feature-with-tests
[Testing Fundamentals]: Introduction to Ruby Tests | Learn RSpec | Testing Fundamentals

I keep getting this error with rspec and I don’t understand why I get it or where it comes from

Failure/Error: expect(response).to render_template(:edit)
       expecting <"edit"> but rendering with <[]>

My spec is this:

context "when user updates with invalid data" do
      let(:person) { Person.create(first_name: "John") }

      it "renders the edit view" do
        patch :update, {id: person.id, person: {fist_name: " "}}

        allow(Person).to receive(:find).
        with(id: person.id).and_return(person)

        allow(person).to receive(:update_attributes).and_return(false)

        expect(response).to render_template(:edit)

      end
    end

And the controller action it is testing:

def update
    @person = Person.find(params[:id])
    if @person.update_attributes(person_attributes)
      redirect_to @person
    else
      render :edit
    end
  end

What am I missing?

I believe the issue lies in the order of the method calls in your spec. Your calling patch before setting up your stud on Person.find, so the method is executing and going through the normal flow.

I would consider all of the stubbing to be “setup” and move it to be first, then the patch line, then the assertion. This would follow the four phase (three in your case) approach to testing. Check out our Weekly Iteration on the Four Phases of Testing for more info.

1 Like

Yeah, that was it. Thank You!

I am having problems with the controller test too. Following the previous post I came up with this:

context "when edit is valid" do
      let(:person) { Person.create(first_name: "John") }

      it "redirects to show" do
        allow(Person).to receive(:find).
        with(id: person.id).and_return(person)
        allow(person).to receive(:update_attributes).and_return(true)

        patch :update, {id: person.id, person: {fist_name: "Johnny"}}
        expect(response).to redirect_to(person_path(person))
      end
    end

The error I get is

Failure/Error: patch :update, {id: person.id, person: {fist_name: "Johnny"}}
       <Person(id: integer, first_name: string) (class)> received :find with unexpected arguments
         expected: ({:id=>1})
              got: ("1")
        Please stub a default value first if message might be received with other args as well.

The argument for Person.find should be an id, not a hash, i.e. allow(Person).to receive(:find).with(person.id)

@andyw8 - When I try your change, I get:

expected: (1)
got: ("1")

Ah yes, the controller will pass the id to Person.find as a string, so you’ll need to convert the person id from an integer to a string to compare them, e.g. .with(person.id.to_s).

Quick functionality question on the unit tests (and I notice the same issue with the previous posts on this topic) - I got a passing test even though my call to PATCH doesn’t contain the correct parameter name for first_name (it’s misspelled):

patch :update, { id: person.id, person: { first_nam: "Hooper" }}

Shouldn’t RSpec or Rails internals be catching this - or have I stumbled into some default behavior which permits undefined parameters to be passed into the controller? FWIW here is what I’ve defined in my controller:

 def update
   @person = Person.find(params[:id])
   if @person.update(person_params)
      redirect_to @person, notice: "Person updated."
   else
     render :edit
   end
 end

 private
   def person_params
     params.require(:person).permit(:first_name)
   end

I noticed that in some solutions people used extract method to create a method to find and set the @person instance variable, I instead went with a before_action.

What could be the proper approach here?

Hi @chischaschos, please excuse me if I’m misunderstanding what you’re describing, but assuming you mean a before_action that will assign the @person instance variable, I would generally avoid that.

In general I prefer to be as explicit as possible, especially when dealing with instance variables as both Ruby and Rails treat them specially. As such, I prefer to extract a method to do the finding, but only assign to the instance variable in the action method. For instance:

class DownloadController < ApplicationController
  def show
    @download = find_download
  end

  private
  
  def find_download
    current_user.downloads.find(params[:id])
  end
end

We actually have a weekly iteration recorded that dives deep on this and related topics, so keep an eye out for that as well.

@christoomey thank you, it answers my question. Is the weekly iteration live?, could you please point me to it?

Not live yet, but it is recorded. I’m giving you a view into the future (cue spooky time travel music here). Not sure specifically when it will come out, but one of the next 3 Fridays is the timeline.

Awesome, really looking forward to see it.

Happy to say the wait is over! Encapsulation and Global State in Rails in now live on the Weekly Iteration.

Yeeii, will look at it today :smile:

Aren’t we supposed to stub Person.create in controller specs?