lib/shoulda/context/context.rb:400: warning: assigned but unused variable - context

[CloCkWeRX]

/home/clockwerx/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-context-1.2.2/lib/shoulda/context/context.rb:400: warning: assigned but unused variable - context

Its obviously not unused; but it might be worth doing something like

original_verbosity = $VERBOSE
$VERBOSE = nil
# context stuff here
$VERBOSE = original_verbosity

[mcmire]

Sorry it took us so long to get to this. This should be fixed in 9b791b4.

[brandur]

It looks like 9b791b4 fixed this, but that change was reverted in 905cb7e. By the commit message, the revert was also supposed to include a fix for the unused variable warning, but I'm still seeing it even on master:

/Users/brandur/stripe/shoulda-context/lib/shoulda/context/context.rb:118: warning: assigned but unused variable - context

Here's a simple fix if you're interested:

diff --git lib/shoulda/context/context.rb lib/shoulda/context/context.rb
index 8c4ee90..b9c78f9 100644
--- lib/shoulda/context/context.rb
+++ lib/shoulda/context/context.rb
@@ -132,6 +132,8 @@ module Shoulda
             end
           end
         end_eval
+
+        context # returned to suppress 'unused variable' warning
       end
 
       def build_test_name_from(should)

Unfortunately, just introducing a plain access of context just produces a different warning ("possibly useless use of a variable in void context"), so this diff moves a use of context down to where the method returns it, therefore giving it a side effect. The method's return value isn't used, so it's still a no-op. Not great, but doesn't have any major downsides that I know of.

[pyrmont]

@brandur This looks like a mistake by Ruby. context is used within the string passed to test_unit_class.class_eval:

shoulda-context/lib/shoulda/context/context.rb

Lines 118 to 134 in dd8c73c

	context = self
	test_unit_class.class_eval <<-end_eval, file, line_no
	define_method test_name do
	@shoulda_context = context
	begin
	context.run_parent_setup_blocks(self)
	if should[:before]
	instance_exec(&should[:before])
	end
	context.run_current_setup_blocks(self)
	instance_exec(&should[:block])
	ensure
	context.run_all_teardown_blocks(self)
	end
	end
	end_eval

Not sure what to do in this case.

[brandur]

@pyrmont I think the problem is that although the code in the block is eventually evaluated by .class_eval, Ruby's interpreter just sees it as a string, and therefore doesn't notice that context gets eventual use.

There may be an argument that Ruby's warning system should be enhanced to allow for that case, but I imagine that might be difficult.

[pyrmont]

One way around this problem is the hack of using 'double assignment'. (This was used in ActiveSupport but has since been changed.)

[brandur]

Cool, I like that better than my proposal. I just tried it, and it seems to do the trick. Patch:

diff --git lib/shoulda/context/context.rb lib/shoulda/context/context.rb
index 8c4ee90..26dbe78 100644
--- lib/shoulda/context/context.rb
+++ lib/shoulda/context/context.rb
@@ -115,7 +115,9 @@ module Shoulda
 
         test_methods[test_unit_class][test_name.to_s] = true
         file, line_no = should[:block].source_location
-        context = self
+
+        # double assignment is used to suppress 'unused variable' warning
+        context = context = self
 
         test_unit_class.class_eval <<-end_eval, file, line_no
           define_method test_name do

[mcmire]

PR to fix this is above! ^

[pyrmont]

I think this issue can be closed, right?

[brandur]

@pyrmont @mcmire Yep, it should be pretty much good to be closed. One thing though — any chance would get a minor release pushed to RubyGems first? I don't think there's been one since #61 came in.

[mcmire]

@brandur Yup, for sure! I've just released 2.0.0.rc4. Btw this will probably be the last pre-release — considering it's been a year since the first pre-release, if everything looks good on your end, I'm just going to pull the trigger.

[brandur]

@mcmire Excellent, thank you! I'll lock us to RC4 in the meantime to put some load on it: stripe/stripe-ruby#910

[mcmire]

Hey @brandur. Now that you've had a chance to try out RC4 for a bit, have you seen any other issues by chance? Wondering if it would be safe to make a proper release.

[brandur]

@mcmire We've been on it for 1.5 months and no one has noticed any issues. I'd +1 going for a full release when you have a chance.