Una mejor alternativa a la hora de escribir/testear rake tasks

rake tasks más amigables.md

El problema

Muchas veces a la hora de programar un script que actualiza muchos datos (tipicamente una rake task), se suele obtener un resultado similar:

Un script largo, procedural, que funciona como una caja negra.

Y el problema es que, para testearlo bien, hay que correrlo muchas veces contra distintos conjuntos de datos, evaluando el estado final de la base de datos. Los tests suelen ser líneas y líneas de setup, pequeñas variaciones entre test y test (muchas veces cambios poco claros o evidentes) y finalmente una evaluación del estado final con una query con gusto a poco.

task :bulk_update_on_records do
  records = []
  SomeClass.joins(other: :tables).where("some conditions").find_each do |record|
    next unless check_record_for_more_conditions

    new_value = # some logic to get the new value
    record.update(column: new_value)
    records << record.id
  end
  puts "Records updated: #{records.count}"
end

Los tests 😢

Cuando se presenta un código así, en general los tests crearán un montón de objectos en la base de datos, correran la tarea y luego deberán recargar los objectos para ver si aquellos que deberían ser afectados tienen un nuevo valor (y lo mismo con aquellos que no deberían cambiar). Esto muchas veces se traduce en varias llamadas a FactoryBot, scopes anidados para las distintas condiciones con sus correspondientes bloques before y tests que solo "ejectuan la tarea" y cuentan los objetos modificados.

let (:object1) { ... }
let (:object2) { ... }
let (:objectN) { ... }

before { ... }

it "check something" do
  task.execute
  expect(SomeClass.where(conditions).count).to eq(1)
end

Alternativa

Esta problemática se puede simplificar facilmente usando POO, separando las distintas responsabilidades en métodos independientes. Primero, pasar el script procedural a una clase que responda a call().

class RecordUpdater
  def call
    # old script goes here
  end
end

Luego se puede separar las distintas partes en métodos individuales. Eso ayuda por dos motivos. El primero, es que nos permite enfocarnos en un solo tema a la vez. El scope reducido ayuda a entender mejor la relación entre datos de entrada y salida. La caja negra se hace cada vez más fácil de comprender. Por otro lado, al ser un problema más pequeño y enfocado, nos permite verificar los casos límite de una manera más detallada.

def fetch_records
  SomeClass.joins(other: :tables).where("some conditions")
end

Con este método podemos asegurarnos más fácil el funcionamiento de la query y ver qué objetos podrían ser modicados luego o no. En este punto todavía no nos interesa saber si cumple todas las condiciones o como se calculan los nuevos valores. Solo queremos saber si nuestra query trae los datos que queremos o no.

def needs_update?(record)
  check_record_for_more_conditions
end

Esta parte ya no depende de la query del paso anterior, por lo que no requiere que los objetos sean traidos de la base de datos. En este punto ya no hace falta persistir los datos para evaluarlos. Lo que antes requería hacer un INSERT para luego ejecutar la query (SELECT...), ahora se puede hacer con objetos en memoria. En el caso del test, lo que antes quizás requería FactoryBot.create(...), ahora se puede hacer con FactoryBot.build(...) o SomeClass.new(...) directamente. Tampoco necesitamos considerar todas las alternativas que evaluaba la query anterior. Solo las que importan a este código en particular.

Así como no necesitamos correr la query para traer los objetos de la base de datos, tampoco necesitamos persistirlos en este paso.

Los siguientes pasos serían extraer la forma de calcular el nuevo valor del objeto y, finalmente, actualizar el método call() para unir las distintas piezas.

Ejemplo

class RecordUpdater
  def fetch_records
    SomeClass.joins(other: :tables).where("some conditions")
  end

  def needs_update?(record)
    record.some_condition? && record.another_condition?
  end

  def calculate_new_value(record)
    # ...
  end

  def update(record)
    new_value = calculate_new_value(record)
    record.update(column: new_value)
  end
  
  def call
    fetch_records.inject([]) do |record|
      if needs_update?(record)
        update(record)
        record.id
      end
    end.compact
  end
end

Para testear el método call() y asegurarnos que todas las partes funcionan bien en conjunto. En este caso, hay dos formas alternativas (aunque complementarias) de testear esto: haciendo un test completo de punta a punta, mockeando o no las partes intermedias.

RSpec.describe RecordUpdater
  context "#fetch_records"
  # checks that fetch_records only returns the records that matters

  context "#needs_update?"
  # checks if the record should be updated or not

  context "#calculate_new_value?"
  # checks the new value that will be updated to

  context "#update"
  # checks that it updates the record with the new value
  
  context "#call" do
    # mock fetch_records(), needs_update() and update() as needed,
    # test that it counts/logs the updated records
    context "with mocks" do
      let(:record1) { build(:record, some_condition: true) }
      let(:record2) { build(:record, some_condition: false) }

      it "checks the ids" do
        allow(subject).to receive(:fetch_records).and_return([record1, record2])
        expect(subject.call).to eq([record1.id])
      end
    end

    # alternative, create some records to see that everything works fine
    context "without mocks" do
      let(:record1) { create(:record, some_condition: true) }
      let(:record2) { create(:record, some_condition: false) }

      it "checks the ids" do
        # we don't mock fetch_records, as they came from the BD this time
        expect(subject.call).to eq([record1.id])
      end
    end
  end
end

Añadido a mis notas personales. Gracias por el aporte!