Surgical Refactors

|item| Suture.create :gilded_rose, :old => lambda { |item| item.update_quality! item }, :args => [item], :record_calls => true end redirect_to items_path end end

None

Plan Cut Record Validate Refactor Verify ⚖ Compare Fallback Delete

✂

def test_validate_old_add calc = Calculator.new Suture.verify :add, subject: calc.method(:add) end

Verifies each recorded args yield the recorded result

def test_validate_old_add calc = Calculator.new Suture.verify :add, subject: calc.method(:add) end

Cheap tests!

def test_old_tally Suture.verify:tally, subject: ->(calc, n){ calc.tally(n) calc.total } end

Duplicate the lambda exactly

Finally, a good use for code coverage!

Trial # 1 Characterization tests

None

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end Items in, mutated items out

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end

def test_gilded_rose_old Suture.verify :rose, subject: ->(items) { update_quality(items) items },

fail_fast: true end All recordings expected to pass

Check coverage before continuing

None

Plan Cut Record Validate Refactor Verify ⚖ Compare Fallback Delete

✂

I'm no refactoring expert That's why I needed this tool

None

Unlike my book, this book exists

class Calculator def add(left, right) right.times do left += 1

end left end end

class Calculator def add(left, right) right.times do left += 1

end left end end Doesn't work for negative values!

class Calculator def new_add(left, right) return left if right <

0 # ^ FIXME later left + right end end

class Calculator def new_add(left, right) return left if right <

0 # ^ FIXME later left + right end end Retain current behavior exactly, bugs & all

class Calculator def new_add(left, right) return left if right <

0 # ^ FIXME later left + right end end We don't know what else depends on bad behavior

class Calculator attr_reader :total def tally(n) @total ||= 0 n.downto(0)

do |i| if i * 2 == n @total += i * 2 end end return end end Skips odd values!

class Calculator def new_tally(n) return if n.odd? # ^ FIXME

later @total ||= 0 @total += n return end end

class Calculator def new_tally(n) return if n.odd? # ^ FIXME

later @total ||= 0 @total += n return end end Still returns nil

class Calculator def new_tally(n) return if n.odd? # ^ FIXME

later @total ||= 0 @total += n return end end "Make the change easy, then make the easy change" - Beck

Plan Cut Record Validate Refactor Verify ⚖ Compare Fallback Delete

✂

def test_new_add calc = Calculator.new Suture.verify :add, subject: calc.method(:new_add) end

✅

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end ❌

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end ❌ ⁉

Judge a library by its messages

None

# Verification of your seam failed! Descriptions of each unsuccessful

verification follows: ## Failures 1.) Recorded call for seam :tally (ID: 13) ran and failed comparison. Arguments: ``` [<Calculator:@total=nil>, 1] ``` Expected returned value: ``` 0 ``` Actual returned value: ``` nil ``` Ideas to fix this: * Focus on this test by setting ENV var `SUTURE_VERIFY_ONLY=13` * Is the recording wrong? Delete it! `Suture.delete!(13)`

1.) Recorded call for seam :tally (ID: 13) ran and

failed comparison. Arguments: ``` [<Calculator:@total=nil>, 1] ``` Expected returned value: ``` 0 ``` Actual returned value: ``` nil ```

Ideas to fix this: * Focus on this test by

setting ENV var `SUTURE_VERIFY_ONLY=13` * Is the recording wrong? Delete it! `Suture.delete!(13)`

Ideas to fix this: * Focus on this test by

setting ENV var `SUTURE_VERIFY_ONLY=13` * Is the recording wrong? Delete it! `Suture.delete!(13)` Only run this failure

Ideas to fix this: * Focus on this test by

setting ENV var `SUTURE_VERIFY_ONLY=13` * Is the recording wrong? Delete it! `Suture.delete!(13)` Delete bad recordings

Failure advice ☹

### Fixing these failures #### Custom comparator If any comparison

is failing and you believe the results are equivalent, we suggest you look into creating a custom comparator. See more details here: https://github.com/testdouble/suture#creating-a-custom-comparator #### Random seed Suture runs all verifications in random order by default. If you're seeing an erratic failure, it's possibly due to order-dependent behavior somewhere in your subject's code. To re-run the tests with the same random seed as was used in this run, set the env var `SUTURE_RANDOM_SEED=74735` or the config entry `:random_seed => 74735`. To re-run the tests without added shuffling (that is, in the order the calls were recorded in), then set the random seed explicitly to nil with env var `SUTURE_RANDOM_SEED=nil` or the config entry `:random_seed => nil`.

### Fixing these failures #### Custom comparator If any comparison

is failing and you believe the results are equivalent, we suggest you look into creating a custom comparator. See more details here: https://github.com/testdouble/ suture#creating-a-custom-comparator

Comparing Results

Default Comparator

==

or

) ( Marshall.dump Marshall.dump == ) (

What about ActiveRecord?!

.attributes == AR AR .attributes

None

!=

Custom Comparators p q r s t

What if Calculator had many other fields? class Calculator attr_reader

:created_at, :memory_val, :total # … end

Suture.verify :tally, subject: ->(calc, n) { calc.new_tally(n) calc.total }, comparator:

->(recorded, actual) { recorded.total == actual.total }

Suture.verify :tally, subject: ->(calc, n) { calc.new_tally(n) calc.total }, comparator:

Classes also exist!

class CalcPare < Suture::Comparator def call(left, right) if super then

return true end left.total == right.total end end Suture.verify :tally, subject: ->(calc, n) { calc.new_tally(n) calc.total }, comparator: CalcPare.new

class CalcPare < Suture::Comparator def call(left, right) if super then

return true end left.total == right.total end end Suture.verify :tally, subject: ->(calc, n) { calc.new_tally(n) calc.total }, comparator: CalcPare.new

class CalcPare < Suture::Comparator def call(left, right) return true if

super left.total == right.total end end Suture.verify :tally, subject: ->(calc, n) { calc.new_tally(n) calc.total }, comparator: CalcPare.new

class CalcPare < Suture::Comparator def call(left, right) return true if

Returning to the error message

#### Random seed Suture runs all verifications in random order

by default. If you're seeing an erratic failure, it's possibly due to order-dependent behavior somewhere in your subject's code. To re-run the tests with the same random seed as was used in this run, set the env var `SUTURE_RANDOM_SEED=74735` or the config entry `:random_seed => 74735`. To re-run the tests without added shuffling (that is, in the order the calls were recorded in), then set the random seed explicitly to nil with env var `SUTURE_RANDOM_SEED=nil` or the config entry `:random_seed => nil`.

#### Random seed Suture runs all verifications in random order

Discoverable configuration

# Configuration This is the configuration used by this test

run: ``` { :comparator => Suture::Comparator.new, :database_path => "db/suture.sqlite3", :fail_fast => false, :call_limit => nil, # (no limit) :time_limit => nil, # (no limit) :error_message_limit => nil, # (no limit) :random_seed => 74735 } ```

# Configuration This is the configuration used by this test

A sense of progress

# Result Summary - Passed........12 - Failed........1 - with error..0

- Skipped.......0 - Total calls...13 ## Progress Here's what your progress to initial completion looks like so far: [••••••••••••••••••••••••••••••••••••••◍◌◌◌◌] Of 13 recorded interactions, 12 are currently passing. That's 92%!

# Result Summary - Passed........12 - Failed........1 - with error..0

- Skipped.......0 - Total calls...13 ## Progress Here's what your progress to initial completion looks like so far: [••••••••••••••••••••••••••••••••••••••◍◌◌◌◌] Of 13 recorded interactions, 12 are currently passing. That's 92%!

Judge a library by its messages

Wait, why did verification fail? ❌

1.) Recorded call for seam :tally (ID: 13) ran and

class Calculator attr_reader :total def new_tally(n) return if n.odd? #

^ FIXME later @total ||= 0 @total += n return end end

class Calculator attr_reader :total def new_tally(n) return if n.odd? #

class Calculator attr_reader :total def new_tally(n) @total ||= 0 return

if n.odd? # ^ FIXME later @total ||= 0 @total += n return end end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end

def test_new_tally Suture.verify :tally, subject: ->(calc, n){ calc.new_tally(n) calc.total }

end ✅

Plan Cut Record Validate Refactor Verify ⚖ Compare Fallback Delete

✂

⚖ Compare

Our mission

Our mission Development

Our mission Development Testing

Our mission Development Testing Staging

Our mission Development Testing Staging Production

Our progress

✅Development Our progress

✅Development ✅Testing Our progress

✅Development ✅Testing ❓Staging Our progress

✅Development ✅Testing ❓Staging ❓Production Our progress

Pure function ❄

class Controller def show calc = Calculator.new @result = Suture.create

:add, old: calc.method(:add), new: calc.method(:new_add), args: [ params[:left], params[:right] ], call_both: true end end

class Controller def show calc = Calculator.new @result = Suture.create

:add, old: calc.method(:add), new: calc.method(:new_add), args: [ params[:left], params[:right] ], call_both: true end end Calls :new & :old

class Controller def show calc = Calculator.new @result = Suture.create

:add, old: calc.method(:add), new: calc.method(:new_add), args: [ params[:left], params[:right] ], call_both: true end end Calls :new & :old ✅

You will find surprising inputs & outputs

Mutation ☣

calc = Calculator.new params[:nums].each {|n| Suture.create :tally, old: ->(my_calc, m)

{ my_calc.tally(m) my_calc.total }, new: ->(my_calc, m) { my_calc.new_tally(m) my_calc.total }, args: [calc, n], call_both: true } @result = calc.total

calc = Calculator.new params[:nums].each {|n| Suture.create :tally, old: ->(my_calc, m)

{ my_calc.tally(m) my_calc.total }, new: ->(my_calc, m) { my_calc.new_tally(m) my_calc.total }, args: [calc, n], call_both: true } @result = calc.total ❌

Another huge error message

The results from the old & new code paths did

not match for the seam (Suture::Error::ResultMismatch) :tally and Suture is raising this error because the `:call_both` option is enabled, because both code paths are expected to return the same result. Arguments: ``` [<Calculator:@total=4>, 2] ``` The new code path returned: ``` 2 ``` The old code path returned: ``` 4 ``` Here's what we recommend you do next: 1. Verify that this mismatch does not represent a missed requirement in the new code path. If it does, implement it! 2. If either (or both) code path has a side effect that impacts the return value of the other, consider passing an `:after_old` and/or `:after_new` hook to clean up your application's state well enough to run both paths one-after-the-other safely. 3. If the two return values above are sufficiently similar for the purpose of your application, consider writing your own custom comparator that relaxes the comparison (e.g. only checks equivalence of the attributes that matter). See the README for more info on custom comparators. 4. If the new code path is working as desired (i.e. the old code path had a bug for this argument and you don't want to reimplement it just to make them perfectly in sync with one another), consider writing a one-off comparator for this seam that will ignore the affected range of arguments. See the README for more info on custom comparators. By default, Suture's :call_both mode will log a warning and raise an error when the results of each code path don't match. It is intended for use in any pre-production environment to "try out" the new code path before pushing it to production. If, for whatever reason, this error is too disruptive and logging is sufficient for monitoring results, you may disable this error by setting `:raise_on_result_mismatch` to false.

Surgical Refactors

Surgical Refactors

More Decks by Justin Searls

Other Decks in Programming

Featured

Transcript