I believe I have found two bugs with Vanity when it is running inside a functional test. Before I submit a Pull Request, I wanted to make sure that what I have found are indeed bugs and not just an test implementation issue.
First off, my test looks something like this:
context "A GET to :foo" do
context "while the 'bar_3' Vanity experiment alternative is chosen" do
setup do
Vanity.playground.experiment(:my_experiment).chooses('bar_3')
get :foo
end
should_render "my_controller/foo/bar_3"
end
end
I ran into two problems with this test case.
- in
lib/vanity/frameworks/rails.rb
, the use_vanity
method initially sets @vanity_identity
to "test" which is later used to store the index of the alternative which has been specifically chosen ("bar_3" in the above example)
The problem occurs once the Controller Action is executed. the use_vanity
method now sees a response object, so it then generates a new @vanity_identity
which will then default to an entirely different test alternative index.
Adding an additional test to see if there is a response and it is not an instance of ActionController::TestResponse
allowed Vanity to correctly set the @vanity_identity
every time.
The real problem might be that @vanity_identity
seems to be losing its value at some point during execution. Not sure where that would be occurring and the vanity_identity
method was run once... so I'm sort of lost on that one.
def use_vanity(symbol = nil, &block)
if block
define_method(:vanity_identity) { block.call(self) }
else
define_method :vanity_identity do
return @vanity_identity if @vanity_identity
if symbol && object = send(symbol)
@vanity_identity = object.id
elsif response and not response.is_a?(ActionController::TestResponse) # everyday use
@vanity_identity = cookies["vanity_id"] || ActiveSupport::SecureRandom.hex(16)
cookies["vanity_id"] = { :value=>@vanity_identity, :expires=>1.month.from_now }
@vanity_identity
else # during functional testing
@vanity_identity = "test"
end
end
end
around_filter :vanity_context_filter
before_filter :vanity_reload_filter unless ::Rails.configuration.cache_classes
before_filter :vanity_query_parameter_filter
end
- in
lib/vanity/experiment/ab_test.rb
, the choose
method is correctly choosing an alternative to display, but it is never saving that value to the index
variable for use later on in the method. That means that it will always choose the first test (since nil.to_i == 0) which is obviously not what was explicitly chosen by the chooses
method.
Adding a simple variable assignment resolved that issue.
def choose
if @playground.collecting?
# snip #
else
identity = identity()
@showing ||= {}
@showing[identity] ||= alternative_for(identity)
index = @showing[identity] # Assign showing value for the current identity to index so the correct Alt. can be displayed
end
@alternatives[index.to_i]
end
Hopefully this was helpful. Please let me know if anymore information is needed to reproduce the bugs, I will be more than happy to provide what I can.
If the above looks good, I will submit a Pull Request once I double check Vanity's tests.