A refactoring tool for Ruby, designed to make it safe to change code you don't confidently understand. In fact, changing untrustworthy code is so fraught, Suture hopes to make it safer to completely reimplement a code path.
Suture provides help to the entire lifecycle of refactoring poorly-understood code, from local development, to a staging environment, and even in production.
Suture was unveiled at Ruby Kaigi 2016 as one approach that can make refactors less scary and more predictable. You can watch the 45 minute screencast here:
Refactoring or reimplementing important code is an involved process! Instead of listing out Suture's API without sufficient exposition, here is an example that will take you through each stage of the lifecycle.
Suppose you have a really nasty worker method:
class MyWorker
def do_work(id)
thing = Thing.find(id)
# … 99 lines of terribleness …
MyMailer.send(thing.result)
end
end
A seam serves as an artificial entry point that sets a boundary around the code you'd like to change. A good seam is:
- easy to invoke in isolation
- takes arguments, returns a value
- eliminates (or at least minimizes) side effects (for more on side effects, see this tutorial)
Then, to create a seam, typically we create a new unit to house the code that we excise from its original site, and then we call it. This adds a level of indirection, which gives us the flexibility we'll need later.
In this case, to create a seam, we might start with this:
class MyWorker
def do_work(id)
MyMailer.send(LegacyWorker.new.call(id))
end
end
class LegacyWorker
def call(id)
thing = Thing.find(id)
# … Still 99 lines. Still terrible …
thing.result
end
end
As you can see, the call to MyMailer.send
is left at the original call site.
MyMailer.send
is effectively a void method being invoked for its side effect,
which would make it difficult to test. By creating LegacyWorker#call
, we can
now express the work more clearly in terms of repeatable inputs (id
) and
outputs (thing.result
), which will help us verify that our refactor is working
later.
Since any changes to the code while it's untested are very dangerous, it's important to minimize changes made for the sake of creating a clear seam.
Next, we introduce Suture to the call site so we can start analyzing its behavior:
class MyWorker
def do_work(id)
MyMailer.send(Suture.create(:worker, {
old: LegacyWorker.new,
args: [id]
}))
end
end
Where old
can be anything callable with call
(like the class above, a
method, or a Proc/lambda) and args
is an array of the args to pass to it.
At this point, running this code will result in Suture just delegating to LegacyWorker without taking any other meaningful action.
Next, we want to start observing how the legacy worker is actually called. What arguments are being sent to it and what value does it returns (or, what error does it raise)? By recording the calls as we use our app locally, we can later test that the old and new implementations behave the same way.
First, we tell Suture to start recording calls by setting the environment
variable SUTURE_RECORD_CALLS
to something truthy (e.g.
SUTURE_RECORD_CALLS=true bundle exec rails s
). So long as this variable is set,
any calls to our seam will record the arguments passed to the legacy code path
and the return value.
As you use the application (whether it's a queue system, a web app, or a CLI), the calls will be saved to a sqlite database. Keep in mind that if the legacy code path relies on external data sources or services, your recorded inputs and outputs will rely on them as well. You may want to narrow the scope of your seam accordingly (e.g. to receive an object as an argument instead of a database id).
If it's difficult to generate realistic usage locally, then consider running this step in production and fetching the sqlite DB after you've generated enough inputs and outputs to be confident you've covered most realistic uses. Keep in mind that this approach means your test environment will probably need access to the same data stores as the environment that made the recording, which may not be feasible or appropriate in many cases.
Next, we should probably write a test that will ensure our new implementation will continue to behave like the old one. We can use these recordings to help us automate some of the drudgery typically associated with writing characterization tests.
We could write a test like this:
class MyWorkerCharacterizationTest < Minitest::Test
def setup
super
# Load the test data needed to resemble the environment when recording
end
def test_that_it_still_works
Suture.verify(:worker, {
:subject => LegacyWorker.new
:fail_fast => true
})
end
end
Suture.verify
will fail if any of the recorded arguments don't return the
expected value. It's a good idea to run this against the legacy code first,
for two reasons:
-
running the characterization tests against the legacy code path will ensure that the test environment has the data needed to behave the same way as when it was recorded (it may be appropriate to take a snapshot of the database before you start recording and load it before you run your tests)
-
by generating a code coverage report (simplecov is a good one to start with) from running this test in isolation, we can see what
LegacyWorker
is actually calling, in an attempt to do two things:- maximize coverage for code in the LegacyWorker (and for code that's subordinate to it) to make sure our characterization test sufficiently exercises it
- identify incidental coverage of code paths that are outside the scope of
what we hope to refactor. This will help to see if
LegacyWorker
has side effects we didn't anticipate and should additionally write tests for
Once the automated characterization test of our recordings is passing, then we
can start work on a NewWorker
. To get started, we update our Suture
configuration:
class MyWorker
def do_work(id)
MyMailer.send(Suture.create(:worker, {
old: LegacyWorker.new,
new: NewWorker.new,
args: [id]
}))
end
end
class NewWorker
def call(id)
end
end
Next, we specify a NewWorker
under the :new
key. For now,
Suture will start sending all of its calls to NewWorker#call
.
Next, let's write a test to verify the new code path also passes the recorded interactions:
class MyWorkerCharacterizationTest < Minitest::Test
def setup
super
# Load the test data needed to resemble the environment when recording
end
def test_that_it_still_works
Suture.verify(:worker, {
subject: LegacyWorker.new,
fail_fast: true
})
end
def test_new_thing_also_works
Suture.verify(:worker, {
subject: NewWorker.new,
fail_fast: false
})
end
end
Obviously, this should fail until NewWorker
's implementation covers all the
cases that we recorded from LegacyWorker
.
Remember, characterization tests aren't designed to be kept around forever. Once you're confident that the new implementation is sufficient, it's typically better to discard them and design focused, intention-revealing tests for the new implementation and its component parts.
This step is the hardest part and there's not much Suture can do to make it any easier. How you go about implementing your improvements depends on whether you intend to rewrite the legacy code path or refactor it. Some comments on each approach follow:
The best time to rewrite a piece of software is when you have a better understanding of the real-world process that it models than the original authors did when they first wrote it. If that's the case, it's likely you'll think of more reliable names and abstractions than they did.
As for workflow, consider writing the new implementation like you would any other
new part of the system. The added benefit is being able to run the
characterization tests as a progress indicator and a backstop for any missed edge
cases. The ultimate goal of this workflow should be to incrementally arrive at a
clean design that completely passes the characterization test run by running
Suture.verify
.
If you choose to refactor the working implementation, though, you should start
by copying it (and all of its subordinate types) into the new, separate code
path. The goal should be to keep the legacy code path in a working state, so
that Suture
can run it when needed until we're supremely confident that it can
be safely discarded. (It's also nice to be able to perform side-by-side
comparisons without having to check out a different git reference.)
The workflow when refactoring should be to take small, safe steps using well understood refactoring patterns and running the characterization test suite frequently to ensure nothing was accidentally broken.
Once the code is factored well enough to work with (i.e. it is clear enough to incorporate future anticipated changes), consider writing some clear and clean unit tests around new units that shook out from the activity. Having good tests for well-factored code is the best guard against seeing it slip once again into poorly-understood "legacy" code.
Once you've changed the code, you still may not be confident enough to delete it entirely. It's possible (even likely) that your local exploratory testing didn't exercise every branch in the original code with the full range of potential arguments and broader state.
Suture gives users a way to experiment with risky refactors by deploying them to
a staging environment and running both the original and new code paths
side-by-side, raising an error in the event they don't return the same value.
This is governed by the :call_both
to true
:
class MyWorker
def do_work(id)
MyMailer.send(Suture.create(:worker, {
old: LegacyWorker.new,
new: NewWorker.new,
args: [id],
call_both: true
}))
end
end
With this setting, the seam will call through to both legacy and refactored implementations, and will raise an error if they don't return the same value. Obviously, this setting is only helpful if the paths don't trigger major or destructive side effects.
You're almost ready to delete the old code path and switch production over to the new one, but fear lingers: maybe there's an edge case your testing to this point hasn't caught.
Suture was written to minimize the inhibition to moving forward with changing code, so it provides a couple features designed to be run in production when you're yet unsure that your refactor or reimplementation is complete.
While your application's logs aren't affected by Suture, it may be helpful for Suture to maintain a separate log file for any errors that are raised by the refactored code path.
Suture has a handful of process-wide logging settings that can be set at any
point as your app starts up (if you're using Rails, then your
environment-specific (e.g. config/environments/production.rb
) config file
is a good choice).
Suture.config({
:log_level => "WARN", #<-- defaults to "INFO"
:log_stdout => false, #<-- defaults to true
:log_io => StringIO.new, #<-- defaults to nil
:log_file => "log/suture.log" #<-- defaults to nil
})
When your new code path raises an error with the above settings, it will propagate and log the error to the specified file.
Additionally, you may have some idea of what you want to do (i.e. phone home to
a reporting service) in the event that your new code path fails. To add custom
error handling before, set the :on_error
option to a callable.
class MyWorker
def do_work(id)
MyMailer.send(Suture.create(:worker, {
old: LegacyWorker.new,
new: NewWorker.new,
args: [id],
on_error: -> (name, args) { PhonesHome.new.phone(name, args) }
}))
end
end
Since the legacy code path hasn't been deleted yet, there's no reason to leave
users hanging if the new code path explodes. By setting the :fallback_on_error
entry to true
, Suture will rescue any errors raised from the new code path and
attempt to invoke the legacy code path instead.
class MyWorker
def do_work(id)
MyMailer.send(Suture.create(:worker, {
old: LegacyWorker.new,
new: NewWorker.new,
args: [id],
fallback_on_error: true
}))
end
end
Since this approach rescues errors, it's possible that errors in the new code path will go unnoticed, so it's best used in conjunction with Suture's logging feature. Before ultimately deciding to finally delete the legacy code path, double-check that the logs aren't full of rescued errors!
Suture.create(name, opts)
- Creates a seam in your production source codeSuture.verify(name, opts)
- Verifies a callable subject can recreate recorded callsSuture.config(config)
- Sets logging options, as well global defaults for other propertiesSuture.reset!
- Resets all Suture configurationSuture.delete!(id)
- Deletes a recorded call byid
Suture.delete_all!(name)
- Deletes all recorded calls for a given seamname
Legacy code is, necessarily, complex and hard-to-wrangle. That's why Suture comes with a bunch of configuration options to modify its behavior, particularly for hard-to-compare objects.
In general, most configuration options can be set in several places:
-
Globally, via an environment variable. The flag
record_calls
will translate to an expected ENV var namedSUTURE_RECORD_CALLS
and can be set from the command line like so:SUTURE_RECORD_CALLS=true bundle exec rails server
, to tell Suture to record all your interactions with your seams without touching the source code. (Note: this is really only appropriate if your codebase only has one Suture seam in progress at a time, since using a global env var configuration for one seam's sake will erroneously impact the other.) -
Globally, via the top-level
Suture.config
method. Most variables can be set via this top-level configuration, likeSuture.config(:database_path => 'my.db')
. Once set, this will apply to all your interactions with Suture for the life of the process until you callSuture.reset!
. -
At a
Suture.create
orSuture.verify
call-site as part of its options hash. If you have several seams, you'll probably want to set most options locally where you call Suture, likeSuture.create(:foo, { :comparator => my_thing })
Suture.create(name, [options hash])
-
name (Required) - a unique name for the seam, by which any recordings will be identified. This should match the name used for any calls to
Suture.verify
by your automated tests -
old - (Required) - something that responds to
call
for the providedargs
of the seam and either is the legacy code path (e.g.OldCode.new.method(:old_path)
) or invokes it (inside an anonymous Proc or lambda) -
args - (Required) - an array of arguments to be passed to the
old
ornew
-
new - like old, but either references or invokes the code path designed to replace the
old
legacy code path. When set, Suture will default to invoking thenew
path at the exclusion of theold
path (unless a mode flag likerecord_calls
,call_both
, orfallback_on_error
suggests differently) -
database_path - (Default:
"db/suture.sqlite3"
) - a path relative to the current working directory to the Sqlite3 database Suture uses to record and playback calls -
record_calls - (Default:
false
) - when set to true, theold
path is called (regardless of whethernew
is set) and its arguments and result (be it a return value or an expected raised error) is recorded into the Suture database for the purpose of more coverage for calls toSuture.verify
. Read more -
call_both - (Default:
false
) - when set to true, thenew
path is invoked, then theold
path is invoked, each with the seam'sargs
. The return value from each is compared with thecomparator
, and if they are not equivalent, then aSuture::Error::ResultMismatch
is raised. Intended after thenew
path is initially developed and to be run in pre-production environments. Read more -
fallback_on_error - (Default:
false
) - designed to be run in production after the initial development of the new code path, when set to true, Suture will invoke thenew
code path. Ifnew
raises an error that isn't anexpected_error_type
, then Suture will invoke theold
path with the same args in an attempt to recover a working state for the user. Read more -
raise_on_result_mismatch - (Default:
true
) - when set to true, thecall_both
mode will merely log incidents of result mismatches, as opposed to raisingSuture::Error::ResultMismatch
-
return_old_on_result_mismatch - (Default:
false
) - when set to true, thecall_both
mode will return the result of theold
code path instead of thenew
code path. This is useful when you want to log mismatches in production (i.e. when you're very confident it's safe and fast enough to usecall_both
in production), but want to fallback to theold
path in the case of a mismatch to minimize disruption to your users -
comparator - (Default:
Suture::Comparator.new
) - determines how return values from the Suture are compared when invokingSuture.verify
or whencall_both
mode is activated. By default, results will be considered equivalent if==
returns true or if theyMarshal.dump
to the same string. If this default isn't appropriate for the return value of your seam, read on -
expected_error_types - (Default:
[]
) - if the seam is expected to raise certain types of errors, don't consider them to be exceptional cases. For example, if your:widget
seam is known to raiseWidgetError
objects in certain cases, setting:expected_error_types => [WidgetError]
will result in:Suture.create
will record expected errors whenrecord_calls
is enabledSuture.verify
will compare recorded and actual raised errors that arekind_of?
any recorded error type (regardless of whetherSuture.verify
is passed a redundant list ofexpected_error_types
)Suture.create
, whenfallback_on_error
is enabled, will allow expected errors raised by thenew
path to propagate, as opposed to logging & rescuing them before calling theold
path as a fallback
-
disable - (Default:
false
) - when enabled, Suture will attempt to revert to the original behavior of theold
path and take no special action. Useful in cases where a bug is discovered in a deployed environment and you simply want to hit the brakes on any new code path experiments by settingSUTURE_DISABLE=true
globally -
dup_args - (Default:
false
) - when enabled, Suture will calldup
on each of the args passed to theold
and/ornew
code paths. Useful when the code path(s) mutate the arguments in such a way as to preventcall_both
orfallback_on_error
from being effective -
after_new - a
call
-able hook that runs afternew
is invoked. Ifnew
raises an error, it is not invoked -
after_old - a
call
-able hook that runs afterold
is invoked. Ifold
raises an error, it is not invoked -
on_new_error - a
call
-able hook that is invoked afternew
raises an unexpected error (seeexpected_error_types
). -
on_old_error - a
call
-able hook that is invoked afterold
raises an unexpected error (seeexpected_error_types
).
Suture.verify(name, [options hash])
Many of the settings for Suture.verify
mirror the settings available to
Suture.create
. In general, the two methods' common options should be configured
identically for a given seam; this is necessary, because the Suture.verify
call
site doesn't depend on (or know about) any Suture.create
call site of the same
name; the only resource they share is the recorded calls in Suture's database.
-
name - (Required) - should be the same name as a seam for which some number of recorded calls exist
-
subject - (Required) - a
call
-able that will be invoked with each recorded set ofargs
and have its result compared to that of each recording. This is used in lieu ofold
ornew
, since the subject of aSuture.verify
test might be either (or neither!) -
database_path - (Default:
"db/suture.sqlite3"
) - as withSuture.create
, a custom database path can be set for almost any invocation of Suture, andSuture.verify is no exception
-
verify_only - (Default:
nil
) - when set to an ID, Suture.verify` will only run against recorded calls for the matching ID. This option is meant to be used to focus work on resolving a single verification failure -
fail_fast - (Default:
false
) -Suture.verify
will, by default, run against every single recording, aggregating and reporting on all errors (just like, say, RSpec or Minitest would). However, if the seam is slow to invoke or if you confidently expect all of the recordings to pass verification,fail_fast
is an appropriate option to set. -
call_limit - (Default:
nil
) - when set to a number, Suture will only verify up to the set number of recorded calls. Because Suture randomizes the order of verifications by default, you can see this as setting Suture.verify to sample a random smattering ofcall_limit
recordings as a smell test. Potentially useful when a seam is very slow -
time_limit - (Default:
nil
) - when set to a number (in seconds), Suture will stop running verifications against recordings oncetime_limit
seconds has elapsed. Useful when a seam is very slow to invoke -
error_message_limit - (Default:
nil
) - when set to a number, Suture will only print up toerror_message_limit
failure messages. That way, if you currently have hundreds of verifications failing, your console isn't overwhelmed by them on each run ofSuture.verify
-
random_seed - (Default: it's random!) - a randomized seed used to shuffle the recordings before verifying them against the
subject
code path. If set tonil
, the recordings will be invoked in insertion-order. If set to a specific number, that number will be used as the random seed (useful when re-running a particular verification failure that can't be reproduced otherwise) -
comparator - (Default:
Suture::Comparator
) - If a custom comparator is used by the seam inSuture.create
, then the same comparator should probably be used bySuture.verify
to ensure the results are comparable. Read more on creating custom comparators ) -
expected_error_types - (Default:
[]
) - this option has little impact onSuture.verify
(since each recording will either verify a return value or an error in its own right), however it can be set to squelch log messages warning that errors were raised when invoking thesubject
-
after_subject - a
call
-able hook that runs aftersubject
is invoked. Ifsubject
raises an error, it is not invoked -
on_new_subject - a
call
-able hook that is invoked aftersubject
raises an unexpected error (seeexpected_error_types
)
Out-of-the-box, Suture will do its best to compare your recorded & actual results
to ensure that things are equivalent to one another, but reality is often less
tidy than a gem can predict up-front. When the built-in equivalency comparator
fails you, you can define a custom one—globally or at each Suture.create
or
Suture.verify
call-site.
If you have a bunch of value types that require special equivalency checks, it makes sense to invest the time to extend built-in one:
class MyComparator < Suture::Comparator
def call(recorded, actual)
if recorded.kind_of?(MyType)
recorded.data_stuff == actual.data_stuff
else
super
end
end
end
So long as you return super
for non-special cases, it should be safe to set an
instance of your custom comparator globally for the life of the process with:
Suture.config({
:comparator => MyComparator.new
})
If a particular seam requires a custom comparator and will always return
sufficiently homogeneous types, it may be good enough to set a custom comparator
inline at the Suture.create
or Suture.verify
call-site, like so:
Suture.create(:my_type, {
:old => method(:old_method),
:args => [42],
:comparator => ->(recorded, actual){ recorded.data_thing == actual.data_thing }
})
Just be sure to set it the same way if you want Suture.verify
to be able to
test your recorded values!
Suture.verify(:my_type, {
:subject => method(:old_method),
:comparator => ->(recorded, actual){ recorded.data_thing == actual.data_thing }
})
Let's face it, a massive proportion of legacy Ruby code in the wild involves
ActiveRecord objects to some extent, and it's important that Suture be equipped
to compare them gracefully. If Suture's default comparator (Suture::Comparator
)
detects two ActiveRecord model instances being compared, it will behave
differently, by this logic:
- Instead of comparing the objects with
==
(which returns true so long as theid
attribute matches), Suture will compare the objects'attributes
hashes instead - The built-in
updated_at
andcreated_at
will typically differ when code is executed at different times and are usually not meaningful to application logic, Suture will ignore these attributes by default
Other attributes may or may not matter (for instance, other timestamp fields,
or the id
of the object), in those cases, you can instantiate the comparator
yourself and tell it which attributes to exclude, like so:
Suture.verify :thing,
:subject => Thing.new.method(:stuff),
:comparator => Suture::Comparator.new(
:active_record_excluded_attributes => [
:id,
:quality,
:created_at,
:updated_at
]
)
If Thing#stuff
returns an instance of an ActiveRecord model, the four
attributes listed above will be ignored when comparing with recorded results.
In all of the above cases, :comparator
can be set on both Suture.create
and
Suture.verify
and typically ought to be symmetrical for most seams.
This repository contains these examples available for your perusal:
Some ideas if you can't get a particular verification to work or if you keep seeing false negatives:
- There may be a side effect in your code that you haven't found, extracted, replicated, or controlled for. Consider contributing to this milestone, which specifies a side-effect detector to be paired with Suture to make it easier to see when observable database, network, and in-memory changes are made during a Suture operation
- Consider writing a custom comparator with a relaxed conception of equivalence between the recorded and observed results
- If a recording was made in error, you can always delete it, either by
dropping Suture's database (which is, by default, stored in
db/suture.sqlite3
) or by observing the ID of the recording from an error message and invokingSuture.delete!(42)