Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marshalize mismatch errors #168

Merged
merged 4 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ Contributions to this project are [released](https://help.github.com/articles/gi
## Submitting a pull request

0. [Fork][fork] and clone the repository
0. Create a new branch: `git checkout -b my-branch-name`
0. Make your change, push to your fork and [submit a pull request][pr]
0. Pat your self on the back and wait for your pull request to be reviewed and merged.
1. Create a new branch: `git checkout -b my-branch-name`
2. Make your change, push to your fork and [submit a pull request][pr]
3. Pat your self on the back and wait for your pull request to be reviewed and merged.

Here are a few things you can do that will increase the likelihood of your pull request being accepted:

Expand Down
23 changes: 23 additions & 0 deletions lib/scientist/experiment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ def raise_on_mismatches?
#
# Returns the configured block.
def before_run(&block)
marshalize(block)
@_scientist_before_run = block
end

Expand All @@ -99,6 +100,7 @@ def behaviors
#
# Returns the configured block.
def clean(&block)
marshalize(block)
@_scientist_cleaner = block
end

Expand Down Expand Up @@ -131,6 +133,7 @@ def clean_value(value)
#
# Returns the block.
def compare(*args, &block)
marshalize(block)
@_scientist_comparator = block
end

Expand All @@ -141,6 +144,7 @@ def compare(*args, &block)
#
# Returns the block.
def compare_errors(*args, &block)
marshalize(block)
@_scientist_error_comparator = block
end

Expand All @@ -159,6 +163,7 @@ def context(context = nil)
#
# This can be called more than once with different blocks to use.
def ignore(&block)
marshalize(block)
@_scientist_ignores ||= []
@_scientist_ignores << block
end
Expand Down Expand Up @@ -251,6 +256,7 @@ def run(name = nil)

# Define a block that determines whether or not the experiment should run.
def run_if(&block)
marshalize(block)
@_scientist_run_if_block = block
end

Expand All @@ -276,6 +282,7 @@ def should_experiment_run?

# Register a named behavior for this experiment, default "candidate".
def try(name = nil, &block)
marshalize(block)
name = (name || "candidate").to_s

if behaviors.include?(name)
Expand All @@ -287,6 +294,7 @@ def try(name = nil, &block)

# Register the control behavior for this experiment.
def use(&block)
marshalize(block)
try "control", &block
end

Expand Down Expand Up @@ -318,4 +326,19 @@ def generate_result(name)
control = observations.detect { |o| o.name == name }
Scientist::Result.new(self, observations, control)
end

private

# In order to support marshaling, we have to make the procs marshalable. Some
# CI providers attempt to marshal Scientist mismatch errors so that they can
# be sent out to different places (logs, etc.) The mismatch errors contain
# code from the experiment. This code contains Procs - which can't be
# marshaled until we run the following code.
def marshalize(block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here explaining why we want to make everything marshalable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Added. 👌

unless block.respond_to?(:_dump) || block.respond_to?(:_dump_data)
def block._dump(_)
to_s
end
end
end
end
21 changes: 21 additions & 0 deletions test/scientist/experiment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,27 @@ def @ex.raised(op, exception)
assert_raises(Scientist::Experiment::MismatchError) { runner.call }
end

it "can be marshaled" do
Fake.raise_on_mismatches = true
@ex.before_run { "some block" }
@ex.clean { "some block" }
@ex.compare_errors { "some block" }
@ex.ignore { false }
@ex.run_if { "some block" }
@ex.try { "candidate" }
@ex.use { "control" }
@ex.compare { |control, candidate| control == candidate }

mismatch = nil
begin
@ex.run
rescue Scientist::Experiment::MismatchError => e
mismatch = e
end

assert_kind_of(String, Marshal.dump(mismatch))
end

describe "#raise_on_mismatches?" do
it "raises when there is a mismatch if the experiment instance's raise on mismatches is enabled" do
Fake.raise_on_mismatches = false
Expand Down