fix testcase: ruby-2.0.0 warned unused variables #8546

Merged
merged 1 commit into from Dec 31, 2012

Projects

None yet

6 participants

@hsbt

ruby 2.0.0 (r38432) later warned unused variables. if you run rake test, some testcase is fail. because this testcase read $stderr with 2.0.0 warning messages.

I think this testcase is enough for confirmation of display warning messages.

@carlosantoniodasilva carlosantoniodasilva and 2 others commented on an outdated diff Dec 18, 2012
actionpack/test/controller/render_test.rb
@@ -1438,7 +1438,7 @@ def test_locals_option_to_assert_template_is_not_supported
get :partial_collection_with_locals
assert_template partial: 'customer_greeting', locals: { greeting: 'Bonjour' }
- assert_equal "the :locals option to #assert_template is only supported in a ActionView::TestCase\n", warning_buffer.string
+ assert_match /the :locals option to #assert_template is only supported in a ActionView::TestCase\n/, warning_buffer.string
@carlosantoniodasilva
carlosantoniodasilva Dec 18, 2012

Thanks, but if I'm not wrong, this is going to issue a warning as well with ambiguous first argument or something like that. Does that work with assert_match and the same string?

@fxn
fxn Dec 18, 2012

But how is this particular change related to unused variables anyway? We want to test string equality there don't we.

@carlosantoniodasilva
carlosantoniodasilva Dec 18, 2012

Yeah. The issue is that the local variables are issuing warnings because they're not used in most of the partials inside the tests, and since a warning is issued, this equality fails. Anyway, you're right, this is more like a workaround than a real solution. And actually, I have a branch here to stop generating some not necessary local variables, I'll check to see if that solves this issue as well and push later this week.

@hsbt
hsbt Dec 18, 2012

@fxn you are right. I think so.

@carlosantoniodasilva thanks for your reply. I'll try to remove unused variables for testcases.

@guilleiguaran
Ruby on Rails member

Can you squash your commits please?

@hsbt

@guilleiguaran I'm sorry. I squashed these commits.

@carlosantoniodasilva 2.0.0 later warned all of unused counter variables in views. I think to fix this problem is so hard.
I only fixed stderr spy point.

@carlosantoniodasilva
Ruby on Rails member

@hsbt Yup I see. I have a fix for part of these warnings, but the counter variables, as you said, are hard to work around. Lets see if we can find anything. Merging this, thanks.

@carlosantoniodasilva
Ruby on Rails member

Actually, @hsbt your change is actually to show the warning, right?

@hsbt

@carlosantoniodasilva it's right.

I tested with bundle exec ruby -w -Itest:lib test/controller/render_test.rb --name test_locals_option_to_assert_template_is_not_supported. (rake test is always added -w option.)

result of before applied my commits.

  1) Failure:
test_locals_option_to_assert_template_is_not_supported(RenderTest) [test/controller/render_test.rb:1441]:
--- expected
+++ actual
@@ -1,2 +1,3 @@
-"the :locals option to #assert_template is only supported in a ActionView::TestCase
+"/path/to/rails/actionpack/test/fixtures/test/_customer_greeting.erb:1: warning: assigned but unused variable - customer_greeting_counter
+the :locals option to #assert_template is only supported in a ActionView::TestCase
 "

result of applied my commits.

# Running tests:

/Users/hsbt/Dropbox/gems/rails/actionpack/test/fixtures/test/_customer_greeting.erb:1: warning: assigned but unused variable - customer_greeting_counter
.
@spastorino spastorino merged commit 5e80b25 into rails:master Dec 31, 2012
@carlosantoniodasilva
Ruby on Rails member

❤️ thanks guys.

@voxik

Not sure what is the origin for the warning, but it has nothing to do with Ruby 2.0, since I observe the same warning with Ruby 1.9.3.

@spastorino
Ruby on Rails member

@voxik the latest patch level of 1.9.3 is making the same thing happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment