Remove RSpec deprecation warnings

[jgaskins]

There is a deprecation warning in RSpec about calling objects defined in let and subject blocks inside a before(:all) block. This PR knocks that out by getting a fresh version of the model (by duping the class) for each spec example. This is so that we can check that the accepted options are set properly by default (the reason that there were expectations in the before(:all) block before).

This also eliminates two unnecessary specs. Before, it was being verified that both the model and its descendant were responding to a message that was being sent to the model in a before hook.

Testing that the model responds to it is unnecessary because if it doesn't it'll fail when it's called. Testing that a subclass responds to a method defined in a superclass is testing that Ruby is actually an object-oriented language. :-)

[solnic]

Thanks Jamie!

I agree that respond_to check was redundant however checking descendants is actually needed since our code manually adds options to descendant classes (see here).

It'd be also nice to replace that Model constant with an anonymous class to avoid polluting global namespace with constants.

[jgaskins]

What I mean is, all subclasses respond to a superset of the messages of its parent class. Without explicitly undefining accept_options or changing it to not be a subclass of Model, can that spec possibly fail?

I'm cool with putting it back, but unless I'm overlooking something it seems more like a sanity check than anything. :-)

It'd be also nice to replace that Model constant with an anonymous class to avoid polluting global namespace with constants.

Went ahead and knocked that out. It was something I was thinking about originally but didn't have a good solution for. I'm still not perfectly happy with this one (I consider let! an RSpec smell), but it does free you up to use the Model constant elsewhere in your tests. :-)

[solnic]

It can break if somebody remove the line where we add options to descendants. w/o this check the spec would pass. What I mean is - we can't rely on ruby's built-in behavior because we manually define options on descendants. Unless I'm missing something O_o

What's wrong with let!?

[jgaskins]

It wasn't checking whether it had the options, just that it had the method. descendant receives the accept_options method from Virtus::Options purely by being a subclass of a class that extends itself with it.

What the spec was doing was essentially the following:

module A
  def a
  end
end

class B
  extend A
end

class C < B
end

… and verifying that C responded to a. But since C is a subclass of B (and we verify that B responds to a by calling it in a before block), this will always be true. Maybe there's magic going on I can't see, but I don't see how it can fail on its own.

What's wrong with let!?

It's not always a bad thing but in this particular case it's creating descendant classes in specs that never use them. I call it a smell because tweaking the structure with describe blocks usually allows you to change them to let blocks so you're only instantiating them when necessary. Maybe using one to describe descendants specifically would help here. I'll have a look at it in a little bit. :-)

[solnic]

Oh sorry. My bad, I was referring to checks for option values, not the method itself, this check obviously makes no sense.