@oinak I should probably write this up more but the tldr is I like the mocking system better and I like custom matchers. There is a lot about RSpec I don’t like though so def a tradeoff.
@davetron5000 I see, thanks for the context.
I think custom matchers are a thing on minitest: https://gist.github.com/willnet/4128828 although I did not miss them and I dislike `allow_any_instance_of(...)`, I think it encourages coupling/discourages proper dependency injection, the only limitation I saw on Minitest::Mock expect was with block params, but not so common to hinder me.
@oinak Yeah, I do allow(SomeClass).to receive(:new).and_return(some_object), and then allow(some_object).to receive just to make it explicit.
At any rate, mocks are a tool in the toolbox for me, not a way of life. I also don't think DI _just_ for testing is good. It’s fine if dependencies need to be configuration, but if they don't, I don't see value in making them configurable when, at least in Ruby, you can mock them by replacing .new.
@davetron5000 For me "DI just for testing" makes a "buried" dependency explicit in the initialization, a new/unfamiliar user of the class looking at initialize realizes that this class needs/uses that other. In 50 line classes it makes little difference, but we don't live in 50 line classes all the time...
@davetron5000 I am also fond of class level methods on collaborators so the client class does *not* need to now if the collaborator object is a class and how to initialize it.
The less they know about each other the easier it gets to change _just_ one of them
@davetron5000 here the injected dependencies may be classes or modules, this orchestator class does not now (or need to).
And I can use a placeholder for reporter until it is implemented.
![require "setup"
require "debug"
class Disburser
include Loggable # provides pseudo-global `logger(msg)`
include Measurable # provides `measure(&blk)`
include Configurable # provides `config(key)`
include Runnable # provides class level `run`
class Error < StandardError; end
def initialize(
loader: ::Disburser::Loader,
backfiller: ::Disburser::Backfill,
reporter: Object.new.tap { def it.run = puts("Reporter.run") }
)
@loader = loader
@backfiller = backfiller
@reporter = reporter
end
def run
logger.info "Disburser.run with options: #{::Setup::Config.instance.config}"
load_csv_files if config(:load_files)
backfill_disbursements if config(:backfill)
run_report
rescue Error => e
abort("Unable to complete: #{e.message}")
end
private
attr_reader :loader, :backfiller, :reporter
def load_csv_files
logger.info "Loading CSV files into DB..."
measure("Load CSV files") { loader.run }
rescue Loader::Error => e
handle_error e
end
def backfill_disbursements
# edited for brevity
end
def run_report
logger.info "Generating disbursement report..."
measure("Report generation") { @report = reporter.run }
rescue Report::Error => e
handle_error e
end
def handle_error(exception)
msg = "[#{exception.class}]: #{exception.message}"
logger.error msg
raise Error, msg
end
end](/proxy/post_attachment/447319/c4bce46bf8.png "require \"setup\"
require \"debug\"
class Disburser
include Loggable # provides pseudo-global `logger(msg)`
include Measurable # provides `measure(&blk)`
include Configurable # provides `config(key)`
include Runnable # provides class level `run`
class Error < StandardError; end
def initialize(
loader: ::Disburser::Loader,
backfiller: ::Disburser::Backfill,
reporter: Object.new.tap { def it.run = puts(\"Reporter.run\") }
)
@loader = loader
@backfiller = backfiller
@reporter = reporter
end
def run
logger.info \"Disburser.run with options: #{::Setup::Config.instance.config}\"
load_csv_files if config(:load_files)
backfill_disbursements if config(:backfill)
run_report
rescue Error => e
abort(\"Unable to complete: #{e.message}\")
end
private
attr_reader :loader, :backfiller, :reporter
def load_csv_files
logger.info \"Loading CSV files into DB...\"
measure(\"Load CSV files\") { loader.run }
rescue Loader::Error => e
handle_error e
end
def backfill_disbursements
# edited for brevity
end
def run_report
logger.info \"Generating disbursement report...\"
measure(\"Report generation\") { @report = reporter.run }
rescue Report::Error => e
handle_error e
end
def handle_error(exception)
msg = \"[#{exception.class}]: #{exception.message}\"
logger.error msg
raise Error, msg
end
end")
@oinak I worked in a large Java Spring app for several years. I liked a lot about this style, but I definitely did not like the flexibility it allowed and having to chase down across several files what code was actually running in production. It's a tradeoff and for me, flexibility is usually bad and not something to strive for. I think the “call .new, then call a named method" is a decent seam and I'm fine with classes being coupled in that way. It makes debugging a lot easier IME.
@davetron5000 a lot of our preferences are based on whatever did hurt to each one of us. I have found RSpec mocking abuse and high class coupling a lot in my job life, and used Minitest and enthusiastic DI in my free time and fun pet projects, and that most probably shaped my biases.
Thanks for sharing the context of your choices, I am sure I will grasp BrutRB better now. 🙇🏼
@oinak Yeah, you are probably right about that - glad to have a nice discussion about it tho!