github / scientist

:microscope: A Ruby library for carefully refactoring critical paths.
MIT License
7.47k stars 442 forks source link

Scientist::Experiment.new creates Scientist::Default instance even when it should have been overridden #92

Open mermop opened 6 years ago

mermop commented 6 years ago

Hi friends! Thank you for your great work with this gem :sparkles:

I am having a problem in Rails apps where Scientist::Experiment defaults to the original Default object until the custom one is called - leading to some head-scratching about why the try block is not running even when enabled? is set to true.

I'm unsure whether this is a problem with the Rails load order because of how I've arranged my files, whether the examples in the README could be a little better, or whether there's genuinely a bug here.

$ bundle exec rails console
Loading development environment (Rails 5.1.6)
irb(main):001:0> Scientist::Experiment.new "something"
=> #<Scientist::Default:0x00007fd667bbcd40 @name="something">
irb(main):002:0> LdapExperiment.new(name: "something")
=> #<LdapExperiment:0x00007fd667b6ee10 @name="something">
irb(main):003:0> Scientist::Experiment.new "something"
=> #<LdapExperiment:0x00007fd667b34968 @name="something">

I've followed the instructions in the README, which are delightful and comprehensive.

# app/experiments/ldap_experiment.rb
require "scientist/experiment"

class LdapExperiment
  include Scientist::Experiment

  attr_accessor :name

  def initialize(name:)
    @name = name
  end

  def enabled?
    # ...
  end

  def publish(result)
    # ...
  end
end

module Scientist::Experiment
  def self.new(name)
    LdapExperiment.new(name: name)
  end
end
# app/models/whatever.rb
class Whatever
  include Scientist
  def do_something
    science "role lookup" do |e|
      e.use { do_one_thing }
      e.try { do_some_other_thing }
    end
  end
end

This is occurring in Rails 3.2.x and Rails 5.1.x applications, with version 1.2.0 of the gem.

mermop commented 6 years ago

For the moment I've moved this block of code into an initializer:

module Scientist::Experiment
  def self.new(name)
    LdapExperiment.new(name: name)
  end
end

Which seems to work, so maybe it's just worth noting that strategy in the README

benterprise commented 5 years ago

I also ran into this problem and got around it by using the initializer above

Linuus commented 5 years ago

Super late response but maybe it helps someone else :)

Rails lazy loads classes in development. So, your experiment file has not been loaded yet and thus, the default experiment has not been overridden. That's why it works after you instantiate your experiment once which forces Rails to load the file.

myhandisout commented 3 years ago

Hi friends! Thank you for your great work with this gem ✨

I am having a problem in Rails apps where Scientist::Experiment defaults to the original Default object until the custom one is called - leading to some head-scratching about why the try block is not running even when enabled? is set to true.

I'm unsure whether this is a problem with the Rails load order because of how I've arranged my files, whether the examples in the README could be a little better, or whether there's genuinely a bug here.

$ bundle exec rails console
Loading development environment (Rails 5.1.6)
irb(main):001:0> Scientist::Experiment.new "something"
=> #<Scientist::Default:0x00007fd667bbcd40 @name="something">
irb(main):002:0> LdapExperiment.new(name: "something")
=> #<LdapExperiment:0x00007fd667b6ee10 @name="something">
irb(main):003:0> Scientist::Experiment.new "something"
=> #<LdapExperiment:0x00007fd667b34968 @name="something">

I've followed the instructions in the README, which are delightful and comprehensive.

# app/experiments/ldap_experiment.rb
require "scientist/experiment"

class LdapExperiment
  include Scientist::Experiment

  attr_accessor :name

  def initialize(name:)
    @name = name
  end

  def enabled?
    # ...
  end

  def publish(result)
    # ...
  end
end

module Scientist::Experiment
  def self.new(name)
    LdapExperiment.new(name: name)
  end
end
# app/models/whatever.rb
class Whatever
  include Scientist
  def do_something
    science "role lookup" do |e|
      e.use { do_one_thing }
      e.try { do_some_other_thing }
    end
  end
end

This is occurring in Rails 3.2.x and Rails 5.1.x applications, with version 1.2.0 of the gem.

benoittgt commented 1 year ago

I did that, and it's ugly

class User < ApplicationRecord

  # Force loading of class otherwise `scientist` load the default
  # experiment class which is not what we want
  DefaultExperiment # our custom class that is enabled withe proper result
  extend Scientist # because it is on class method

end