How mutation testing causes deeper thinking about your code + constructor for an included module in Ruby

This is a short story which starts with being very surprised by mutation testing results and trying to figure out how to deal with it.

Consider this module from aggregate_root gem which is part of Rails Event Store.

module AggregateRoot
  def apply(event)
    apply_strategy.(self, event)
    unpublished_events << event
  end

  def load(stream_name, event_store: default_event_store)
    @loaded_from_stream_name = stream_name
    events = event_store.read_stream_events_forward(stream_name)
    events.each do |event|
      apply(event)
    end
    @version = events.size - 1
    @unpublished_events = []
    self
  end

  def store(stream_name = loaded_from_stream_name, event_store: default_event_store)
    event_store.publish_events(
      unpublished_events,
      stream_name: stream_name,
      expected_version: version
    )
    @version += unpublished_events.size
    @unpublished_events = []
  end

  private
  attr_reader :loaded_from_stream_name

  def unpublished_events
    @unpublished_events ||= []
  end

  def version
    @version ||= -1
  end
end

In two places it uses @unpublished_events = [].

However, besides normal specs, we also check out mutation testing coverage using mutant.

If you are not familiar with the technique, in short, it works like this:

change the code subtly to introduce an incorrect behavior
verify if the specs failed

If the specs continue passing, it might indicate that you have a missing test.

However, one thing we realized over time is that mutant often tries to tell you something deeper and point out a potential, higher level problem with your design. With every mutation detected it’s good to dig a bit deeper and ask yourself why 5 times :)

Let’s discuss a simple example.

Mutant decided to change

@unpublished_events = []

into

@unpublished_events = nil

Introducing nil in random places is a great technique to discover untested or unused code. After all, if changing an assignment to nil does not break your code, why do you need it at all? Either you don’t need it and you can remove the line of code, or you miss a spec that properly verifies this line of code.

My first reaction was fuck you, that doesn’t make any sense. Why would you change an empty array to a nil?

I think about @unpublished_events that it is an Array. You can see that in

unpublished_events << event

and in:

def unpublished_events
  @unpublished_events ||= []
end

so fuck off mutant, mkay? 😤

Then I calmed down and started thinking about it. 😜

I changed the code manually from:

@unpublished_events = []

@unpublished_events = nil

I run the tests and they passed. I don’t know what I was thinking, obviously, they passed, mutant already verified that they pass in such case. That’s why I am here. But I didn’t believe so I made this change manually and of course specs passed. I was confused. I looked into those specs to find out if we were missing some cases and we did not.

So why was everything working? - I wondered. And I quickly realized.

Because of the getter with a default Array:

def unpublished_events
  @unpublished_events ||= []
end

3 places in this code which need @unpublished_events always access them via this getter:

unpublished_events << event

@version += unpublished_events.size

So even though @unpublished_events is nil, it will work because upon reading it will become an empty array and work nicely with << and size methods.

def unpublished_events
  @unpublished_events ||= []
end

Then I asked myself… Why are we using this unpublished_events getter at all? Why do we even need it? Why not use @unpublished_events everywhere?

And the answer was… Because we don’t set @unpublished_events = [] in a constructor.

You see, usually, you use the library in two ways. You load historical domain events when you edit an object.

product = Product.new
product.load("Product$#{sku}") # this sets
                               # @unpublished_events

# usually we verify business rules before that step
product.apply(ProductReserved.new(
  quantity: quantity,
  order_number: order_number
))
product.store

or we may skip loading historical domain events for new records and go straight into changing their state and saving in DB.

product = Product.new # @unpublished_events is nil
product.apply(ProductRegistered.new(
  sku: sku,
))
product.store("Product$#{sku}")

In this second case, we want to append new domain events to @unpublished_events collection but it is a nil. Using the unpublished_events getter workarounds this problem.

unpublished_events << event

Ok, so we don’t set @unpublished_events = [] in a constructor. But why? Why don’t we do that?

Because AggregateRoot is a module and not a class.

class Product
  include AggregateRoot
end

This led me to next two questions:

should AggregateRoot be a module that you include or a class to inherit from?

I prefer that it is a module.
can we have constructors for modules?

It turns out we can with the little help of prepend which is available in Ruby for years now. Check it out.

module AggregateRoot
  module Constructor
    def initialize(*vars, &proc)
      @unpublished_events = []
      super
    end
  end
  def self.included(klass)
    klass.prepend(Constructor)
  end

  def apply(event)
    @unpublished_events << event
  end
end

class Product
  include AggregateRoot
end

You might be thinking why not simply:

module AggregateRoot
  def initialize(*vars, &proc)
    @unpublished_events = []
    super
  end

  def apply(event)
    @unpublished_events << event
  end
end

But there are some problems:

another developer might forget to call super in a class constructor to trigger AggregateRoot#initialize and @unpublished_events will be nil.

class Product
  include AggregateRoot
  def initialize(dependency)
    @dep = dependency
    # missing super
  end
end

Inside AggregateRoot#initialize we don’t know how many arguments we should provide for a parent class constructor

module AggregateRoot
  def initialize(*vars, &proc)
    @unpublished_events = []
    super
  end

  def apply(event)
    @unpublished_events << event
  end
end

class Product
  include AggregateRoot

  def initialize(dependency)
    @dep = dependency
    super
  end
end

Product.new(1)

# ArgumentError: wrong number of arguments (given 1, expected 0)
# from (irb):4:in  `initialize'
# from (irb):4:in  `initialize'
# from (irb):17:in `initialize'
# from (irb):21:in `new'

If we try to workaround the previous problem by not calling super from our module we get into problems in different situations.

module AggregateRoot
  def initialize(*vars, &proc)
    @unpublished_events = []
    # super
  end

  def apply(event)
    @unpublished_events << event
  end
end

class Something
  def initialize(dependency1)
    @dep1 = dependency1
  end
end

class Product < Something
  include AggregateRoot

  def initialize(dependency1, dependency2)
    @dep2 = dependency2
    super(dependency1)
  end
end

p = Product.new(:one, :two)
p.instance_variable_get(:@dep1)
# => nil

It seems to me that while prepending Constructor can work it does not seem to be very intuitive.

module AggregateRoot
  module Constructor
    def initialize(*vars, &proc)
      @unpublished_events = []
      super
    end
  end
  def self.included(klass)
    klass.prepend(Constructor)
  end

  def apply(event)
    @unpublished_events << event
  end
end

class Product
  include AggregateRoot
end

If I had many instance variables to set I could consider it. But with one or two, I think I am gonna stay with a getter and a default value.

def unpublished_events
  @unpublished_events ||= []
end

How mutation testing causes deeper thinking about your code + constructor for an included module in Ruby

You might also like