PiRSquared17 / activescaffold

Automatically exported from code.google.com/p/activescaffold
MIT License
0 stars 0 forks source link

NoMethodError during functional testing of ActiveScaffold controllers #765

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
If I have a model which has a belongs_to foreign key assocation to another 
model:

  class Reply < ActiveRecord::Base
    belongs_to :person
    belongs_to :event
    belongs_to :availability

    validates_presence_of :person_id, :event_id
    validates_uniqueness_of :person_id, :scope => :event_id, \
      :message => "has already replied for this event"

    [... snip ...]

  end

and I write the following functional test for my AS controller:

  require File.dirname(__FILE__) + '/../test_helper'

  class RepliesControllerTest < ActionController::TestCase

    test "should_create_reply" do
      new_record = {
        :person       => { :id => people(:fred).id  },
        :event        => { :id => events(:concert).id },
        :availability => { :id => availabilities(:yep).id },
      }
      assert_difference('Reply.count') do
        post :create, { :commit => "Create", :record => new_record }
      end

      assert_redirected_to replies_path
    end
  end

then I get the following error:

  1) Error:
test_should_create_reply(RepliesControllerTest):
NoMethodError: undefined method `empty?' for 4:Fixnum
    /test/functional/replies_controller_test.rb:24:in `test_should_create_reply'
    /test/functional/replies_controller_test.rb:23:in `test_should_create_reply'

The test should pass, but lib/active_scaffold/attribute_params.rb
is not correctly handling singular associations.  The guilty code is:

        if column.form_ui == :select
          ids = if column.singular_association?
            value[:id]
          else
            value.values.collect {|hash| hash[:id]}
          end
          (ids and not ids.empty?) ? column.association.klass.find(ids) : nil

but if column.singular_association? is true, ids ends up being a
Fixnum not an Array, so the invocation of the empty? method causes the
NoMethodError shown above.

Worse still, the failure gives no indication of why the failure
happened.  It took me a lot of debugging and research before I found
that the cause of the error could be pinpointed via:

  BACKTRACE=yes rake test:functionals

This was using git changeset c8c85e61d8b4deacee28787548576e143c4c5e35.
I'm currently unable to try with a more recent changeset, because it
would require upgrading my Rails installation which would introduce
breakage in an unrelated area.  However I strongly suspect the issue
is still there in the master branch.

I have produced a fix, and will issue a pull request for it shortly.

Original issue reported on code.google.com by adam.spi...@gmail.com on 26 Sep 2010 at 11:20

GoogleCodeExporter commented 9 years ago
Test is not valid, when you submit from a browser values are strings, so you 
never will get a Fixnum in that line.

Original comment by sergio.c...@gmail.com on 27 Sep 2010 at 8:17

GoogleCodeExporter commented 9 years ago
Hmm, I see.  It is extremely difficult to find the right test code, because 
there is no documentation for it.  I produced that test code after hours of 
searching the web and AS forums for working examples of controller tests.  It 
works if I change it to

      new_record = {
        :person       => { :id => people(:fred).id.to_s  },
        :event        => { :id => events(:concert).id.to_s },
        :availability => { :id => availabilities(:yep).id.to_s },
      }

but this is ugly, so I still think it would be worth applying my patch.

To answer your question at 
http://github.com/activescaffold/active_scaffold/pull/54#issuecomment-431603,
the controller code is:

  active_scaffold :reply do |config|
    config.actions.exclude :show

    config.columns = [ :event, :person, :availability, :comment ]
    config.columns[:availability].label = "Availability"
    config.columns[:availability].form_ui = :select

    config.columns[:event].form_ui = :select
    config.columns[:event].includes = [ 'event' ]

    config.columns[:person].form_ui = :select

    [... snip ...]
  end

Original comment by adam.spi...@gmail.com on 27 Sep 2010 at 2:10

GoogleCodeExporter commented 9 years ago
It is worth noting that I am not the only person who has had difficulty with 
this.  For example:

http://groups.google.com/group/activescaffold/browse_thread/thread/484568a64beb8
beb/d11b869ee6d8fc82?#d11b869ee6d8fc82

Original comment by adam.spi...@gmail.com on 27 Sep 2010 at 2:15

GoogleCodeExporter commented 9 years ago
It's ugly but you mustn't test with a non-real scenario. I have tested 
controllers using other gems and tests failed if I didn't use a string in 
parameters. Why can't we use string methods if in a real application it will be 
a string? Also, you could change .id.to_s with .to_param which should return a 
string IIRC.

About code you are testing, what are the field names you get in the form? Do 
you get record[event] or record[event][id]? I think record[event][id] is not 
used anymore, so your test should use:
      new_record = {
        :person       => people(:fred).to_param,
        :event        => events(:concert).to_param,
        :availability => availabilities(:yep).to_param
      }

Also, you aren't testing only your controller code, you are testing 
ActiveScaffold. You should test your code, such as some columns use form_ui or 
helper override, form includes or excludes some columns and so on.

The linked thread is 2 years old, and is testing with collection param and 
collections instance variable, which are not used in ActiveScaffold. It has 
nothing to see with this report.

Original comment by sergio.c...@gmail.com on 27 Sep 2010 at 3:04

GoogleCodeExporter commented 9 years ago
OK, thanks for the information.  It would be a MASSIVE help if it was 
documented on the wiki how to write controller tests like this.  Currently it 
involves a lot of searching of the group archives and intelligent guesswork or 
reading the AS source code.

Original comment by adam.spi...@gmail.com on 27 Nov 2010 at 12:08

GoogleCodeExporter commented 9 years ago
Just in case anyone else struggling with this arrives here from google, here is 
the winning formula:

  test "should_create_reply" do
    new_record = {
      :person => people(:fred).id.to_param,
      :event  => events(:concert).id.to_param,
      :availability => availabilities(:maybe).id.to_param
    }
    assert_difference('Reply.count') do
      post :create, { :commit => "Create", :record => new_record }
    end
    assert_redirected_to replies_path
  end

  test "should_update_reply" do
    updated_record = {
      :availability => availabilities(:yep).id.to_param
    }
    put :update, {
      :commit => "Update",
      :id => replies(:joe_reply_for_concert).id,
      :record => updated_record
    }
    assert_redirected_to replies_path
  end

Original comment by adam.spi...@gmail.com on 27 Nov 2010 at 1:05

GoogleCodeExporter commented 9 years ago
You can add to wiki a "Testing entry" with this info

Original comment by sergio.c...@gmail.com on 29 Nov 2010 at 8:48