diff --git a/.deepsource.toml b/.deepsource.toml new file mode 100644 index 0000000..7f6697b --- /dev/null +++ b/.deepsource.toml @@ -0,0 +1,9 @@ +version = 1 + +[[analyzers]] +name = "ruby" +enabled = true + +[[analyzers]] +name = "shell" +enabled = true \ No newline at end of file diff --git a/lib/scientist/experiment.rb b/lib/scientist/experiment.rb index f48b339..3a2e7ae 100644 --- a/lib/scientist/experiment.rb +++ b/lib/scientist/experiment.rb @@ -28,7 +28,7 @@ def self.set_default(klass) end # A mismatch, raised when raise_on_mismatches is enabled. - class MismatchError < Exception + class MismatchError < RuntimeError attr_reader :name, :result def initialize(name, result) @@ -130,7 +130,7 @@ def clean_value(value) # and return true or false. # # Returns the block. - def compare(*args, &block) + def compare(*_args, &block) @_scientist_comparator = block end @@ -140,7 +140,7 @@ def compare(*args, &block) # and return true or false. # # Returns the block. - def compare_errors(*args, &block) + def compare_errors(*_args, &block) @_scientist_error_comparator = block end @@ -202,7 +202,7 @@ def raise_with(exception) # Called when an exception is raised while running an internal operation, # like :publish. Override this method to track these exceptions. The # default implementation re-raises the exception. - def raised(operation, error) + def raised(_operation, error) raise error end @@ -290,6 +290,13 @@ def use(&block) try "control", &block end + # Define a block which will determine the cohort of this experiment + # when called. The block will be passed a `Scientist::Result` as its + # only argument and the cohort will be set on the result. + def cohort(&block) + @_scientist_determine_cohort = block + end + # Whether or not to raise a mismatch error when a mismatch occurs. def raise_on_mismatches? if raise_on_mismatches.nil? @@ -316,7 +323,7 @@ def generate_result(name) end control = observations.detect { |o| o.name == name } - Scientist::Result.new(self, observations, control) + Scientist::Result.new(self, observations, control, @_scientist_determine_cohort) end private diff --git a/lib/scientist/result.rb b/lib/scientist/result.rb index 76a4d21..79dc43b 100644 --- a/lib/scientist/result.rb +++ b/lib/scientist/result.rb @@ -19,19 +19,33 @@ class Scientist::Result # An Array of Observations in execution order. attr_reader :observations + # If the experiment was defined with a cohort block, the cohort this + # result has been determined to belong to. + attr_reader :cohort + # Internal: Create a new result. # - # experiment - the Experiment this result is for - # observations: - an Array of Observations, in execution order - # control: - the control Observation + # experiment - the Experiment this result is for + # observations: - an Array of Observations, in execution order + # control: - the control Observation + # determine_cohort - An optional callable that is passed the Result to + # determine its cohort # - def initialize(experiment, observations = [], control = nil) + def initialize(experiment, observations = [], control = nil, determine_cohort = nil) @experiment = experiment @observations = observations @control = control @candidates = observations - [control] evaluate_candidates + if determine_cohort + begin + @cohort = determine_cohort.call(self) + rescue StandardError => e + experiment.raised :cohort, e + end + end + freeze end diff --git a/test/scientist/experiment_test.rb b/test/scientist/experiment_test.rb index 12462f6..de859c1 100644 --- a/test/scientist/experiment_test.rb +++ b/test/scientist/experiment_test.rb @@ -149,7 +149,7 @@ def ex.enabled? true end - def ex.publish(result) + def ex.publish(_result) raise "boomtown" end @@ -164,7 +164,7 @@ def ex.publish(result) end it "reports publishing errors" do - def @ex.publish(result) + def @ex.publish(_result) raise "boomtown" end @@ -288,7 +288,7 @@ def @ex.enabled? end it "reports an error and returns the original value when an error is raised in a clean block" do - @ex.clean { |value| raise "kaboom" } + @ex.clean { |_value| raise "kaboom" } @ex.use { "control" } @ex.try { "candidate" } @@ -302,6 +302,46 @@ def @ex.enabled? assert_equal "kaboom", exception.message end + describe "cohorts" do + it "accepts a cohort config block" do + @ex.cohort { "1" } + end + + it "assigns a cohort to the result using the provided block" do + @ex.context(foo: "bar") + @ex.cohort { |res| "foo-#{res.context[:foo]}-#{Math.log10(res.control.value).round}" } + @ex.use { 5670 } + @ex.try { 5670 } + + @ex.run + assert_equal "foo-bar-4", @ex.published_result.cohort + end + + it "assigns no cohort if no cohort block passed" do + @ex.use { 5670 } + @ex.try { 5670 } + + @ex.run + assert_nil @ex.published_result.cohort + end + + it "rescues errors raised in the cohort determination block" do + @ex.use { 5670 } + @ex.try { 5670 } + @ex.cohort { |_res| raise "intentional" } + + @ex.run + + refute_nil @ex.published_result + assert_nil @ex.published_result.cohort + + assert_equal 1, @ex.exceptions.size + code, exception = @ex.exceptions[0] + assert_equal :cohort, code + assert_equal "intentional", exception.message + end + end + describe "#raise_with" do it "raises custom error if provided" do CustomError = Class.new(Scientist::Experiment::MismatchError) @@ -372,9 +412,9 @@ def @ex.enabled? it "calls multiple ignore blocks to see if any match" do called_one = called_two = called_three = false - @ex.ignore { |a, b| called_one = true; false } - @ex.ignore { |a, b| called_two = true; false } - @ex.ignore { |a, b| called_three = true; false } + @ex.ignore { |_a, _b| called_one = true; false } + @ex.ignore { |_a, _b| called_two = true; false } + @ex.ignore { |_a, _b| called_three = true; false } refute @ex.ignore_mismatched_observation?(@a, @b) assert called_one assert called_two @@ -383,9 +423,9 @@ def @ex.enabled? it "only calls ignore blocks until one matches" do called_one = called_two = called_three = false - @ex.ignore { |a, b| called_one = true; false } - @ex.ignore { |a, b| called_two = true; true } - @ex.ignore { |a, b| called_three = true; false } + @ex.ignore { |_a, _b| called_one = true; false } + @ex.ignore { |_a, _b| called_two = true; true } + @ex.ignore { |_a, _b| called_three = true; false } assert @ex.ignore_mismatched_observation?(@a, @b) assert called_one assert called_two @@ -452,7 +492,7 @@ def @ex.raised(op, exception) @ex.clean { "So Clean" } err = assert_raises(Scientist::Experiment::MismatchError) { @ex.run } - assert_match /So Clean/, err.message + assert_match(/So Clean/, err.message) end it "doesn't raise when there is a mismatch if raise on mismatches is disabled" do diff --git a/test/scientist/observation_test.rb b/test/scientist/observation_test.rb index b788663..e58105c 100644 --- a/test/scientist/observation_test.rb +++ b/test/scientist/observation_test.rb @@ -145,7 +145,7 @@ end it "doesn't clean nil values" do - @experiment.clean { |value| "foo" } + @experiment.clean { |_value| "foo" } a = Scientist::Observation.new("test", @experiment) { nil } assert_nil a.cleaned_value end diff --git a/test/scientist/result_test.rb b/test/scientist/result_test.rb index c9bc41e..9982718 100644 --- a/test/scientist/result_test.rb +++ b/test/scientist/result_test.rb @@ -80,7 +80,7 @@ y = Scientist::Observation.new("y", @experiment) { :y } z = Scientist::Observation.new("z", @experiment) { :z } - @experiment.ignore { |control, candidate| candidate == :y } + @experiment.ignore { |_control, candidate| candidate == :y } result = Scientist::Result.new @experiment, [x, y, z], x @@ -98,6 +98,17 @@ assert_equal @experiment.name, result.experiment_name end + it "takes an optional callable to determine cohort" do + a = Scientist::Observation.new("a", @experiment) { 1 } + b = Scientist::Observation.new("b", @experiment) { 1 } + + result = Scientist::Result.new @experiment, [a, b], a + assert_nil result.cohort + + result = Scientist::Result.new @experiment, [a, b], a, ->(_res) { "cohort-1" } + assert_equal "cohort-1", result.cohort + end + it "has the context from an experiment" do @experiment.context :foo => :bar a = Scientist::Observation.new("a", @experiment) { 1 } diff --git a/test/scientist_test.rb b/test/scientist_test.rb index 5e27026..c784650 100644 --- a/test/scientist_test.rb +++ b/test/scientist_test.rb @@ -28,7 +28,7 @@ obj = Object.new obj.extend(Scientist) - assert_equal Hash.new, obj.default_scientist_context + assert_equal({}, obj.default_scientist_context) end it "respects default_scientist_context" do