Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ You can generate a data migration as you would a schema migration:
rake db:structure:load:with_data # Load both structure.sql and data_schema.rb file into the database
rake db:version:with_data # Retrieves the current schema version numbers for data and schema migrations


Tasks work as they would with the 'vanilla' db version. The 'with_data' addition to the 'db' tasks will run the task in the context of both the data and schema migrations. That is, rake db:rollback:with_data will check to see if it was a schema or data migration invoked last, and do that. Tasks invoked in that space also have an additional line of output, indicating if the action is performed on data or schema.

With 'up' and 'down', you can specify the option 'BOTH', which defaults to false. Using true, will migrate both the data and schema (in the desired direction) if they both match the version provided. Again, going up, schema is given precedence. Down its data.
Expand Down Expand Up @@ -126,10 +125,24 @@ DataMigrate.configure do |config|
'password' => nil,
}
config.spec_name = 'primary'
end

# Enable data_migration generator to create test files
config.test_support_enabled = true # default: false
config.test_framework = :rspec # default: nil (will infer framework when test support enabled)
end
```

### Test Suite Support

When `config.test_support_enabled = true`, the `data_migration` generator will create test files for your data migrations. You can explitcly set the test framework from the `config.test_framework` setting. Otherwise, this will be inferred from the detected test suite.
the test suite you are using.

The standard data migration Rake task will now utilize corresponding RSpec and Minitest templates.
For example, if you run `rails g data_migration add_this_to_that`, the following files will be created:

- `/spec/db/data/add_this_to_that_spec.rb` (for RSpec)
- `/test/db/data/add_this_to_that_test.rb` (for Minitest)

## Capistrano Support

The gem comes with a capistrano task that can be used instead of `capistrano/rails/migrations`.
Expand Down
1 change: 1 addition & 0 deletions lib/data_migrate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
require File.join(File.dirname(__FILE__), "data_migrate", "status_service")
require File.join(File.dirname(__FILE__), "data_migrate", "migration_context")
require File.join(File.dirname(__FILE__), "data_migrate", "railtie")
require File.join(File.dirname(__FILE__), "data_migrate", "helpers/infer_test_suite_type")
require File.join(File.dirname(__FILE__), "data_migrate", "tasks/data_migrate_tasks")
require File.join(File.dirname(__FILE__), "data_migrate", "config")
require File.join(File.dirname(__FILE__), "data_migrate", "schema_migration")
Expand Down
12 changes: 10 additions & 2 deletions lib/data_migrate/config.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module DataMigrate
include ActiveSupport::Configurable
class << self

class << self
def configure
yield config
end
Expand All @@ -12,7 +12,7 @@ def config
end

class Config
attr_accessor :data_migrations_table_name, :data_migrations_path, :data_template_path, :db_configuration, :spec_name
attr_accessor :data_migrations_table_name, :data_migrations_path, :data_template_path, :db_configuration, :spec_name, :test_support_enabled, :test_framework

DEFAULT_DATA_TEMPLATE_PATH = "data_migration.rb"

Expand All @@ -22,12 +22,20 @@ def initialize
@data_template_path = DEFAULT_DATA_TEMPLATE_PATH
@db_configuration = nil
@spec_name = nil
@test_support_enabled = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick:

I've read your PR a few times so far. Decided to do one last round from the "does it make me confused if I jump straight into the code without reading the context?". So please consider my comments as ideas / suggestions, and not something authoritative.

In this instance, I feel like the name is a bit confusing. Because it's not that we turn on test support, but we turn on test generation. So I'd prefer to have test_generator_enabled, generate_tests, generate_specs, generate_test_files, etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this suggested change as it better represents what the configuration value achieves 💯

@test_framework = nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick:

It feels like the default shouldn't be nil, but we should attempt to infer rspec/minitest early.

I'm not sure it makes as much sense from the "config lifecycle" perspective, though. If not, I wonder if we can make the limitation a bit more obvious?

Copy link
Author

@joshmfrankel joshmfrankel Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we were to push the InferTestSuiteType call up into the configuration initializer here? That way, inference check would only happen once (at configuration level), and we could move the early return out of the inference class entierly. Setting the test_framework would then override the inferred value for manual control over the generator.

Also, I'd like to update InferTestSuiteType => InferTestFramework to match the configuration value naming

# lib/data_migrate/config.rb
def initialize
  @data_migrations_table_name = "data_migrations"
  @data_migrations_path = "db/data/"
  @data_template_path = DEFAULT_DATA_TEMPLATE_PATH
  @db_configuration = nil
  @spec_name = nil
  @test_support_enabled = false
  @test_framework = DataMigrate::Helpers::InferTestFramework.new.call
end

# lib/data_migrate/helpers/infer_test_framework.rb
module DataMigrate
  module Helpers
    class InferTestFramework
      def call
        if File.exist?(Rails.root.join('spec', 'spec_helper.rb'))
          :rspec
        elsif File.exist?(Rails.root.join('test', 'test_helper.rb'))
          :minitest
        else
          raise StandardError.new('Unable to determine test suite')
        end
      end
    end
  end
end

# lib/generators/data_migration/data_migration_generator.rb
def create_data_migration_test
  return unless DataMigrate.config.test_support_enabled

  case DataMigrate.config.test_framework
  when :rspec
    template "data_migration_spec.rb", data_migrations_spec_file_path
  when :minitest
    template "data_migration_test.rb", data_migrations_test_file_path
  end
end

I believe this cleans up the logic and keeps a good separation of responsibility throughout the flow. Thoughts on the above?

end

def data_template_path=(value)
@data_template_path = value.tap do |path|
raise ArgumentError, "File not found: '#{path}'" unless path == DEFAULT_DATA_TEMPLATE_PATH || File.exist?(path)
end
end

def test_framework=(value)
raise ArgumentError, "Invalid test framework: #{value}" unless [:rspec, :minitest].include?(value)

@test_framework = value
end
end
end
17 changes: 17 additions & 0 deletions lib/data_migrate/helpers/infer_test_suite_type.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module DataMigrate
module Helpers
class InferTestSuiteType
def call
return DataMigrate.config.test_framework if DataMigrate.config.test_framework.present?

if File.exist?(Rails.root.join('spec', 'spec_helper.rb'))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could perform add to the conditionals here for Rails.configuration.generators.options[:rails][:test_framework] == :rspec and Rails.configuration.generators.options[:rails][:test_framework] == :test_unit respectively to look for both installation and configuration.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of getting the test framework from Rails's config. I've worked on projects where both rspec and minitest were used (migration happening but not over and very little bandwidth to finish it) and I think its easier for us to maintain compatibility with the configuration over the presence of files?

Perhaps adding a config line would work too, but I'd imagine projects running on sinatra for example to maybe not follow the conventions set elsewhere?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a simple configuration value to early return when set OR fallback to the inference logic.

:rspec
elsif File.exist?(Rails.root.join('test', 'test_helper.rb'))
:minitest
else
raise StandardError.new('Unable to determine test suite')
end
end
end
end
end
24 changes: 24 additions & 0 deletions lib/generators/data_migration/data_migration_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,30 @@ class DataMigrationGenerator < Rails::Generators::NamedBase
def create_data_migration
set_local_assigns!
migration_template template_path, data_migrations_file_path
create_data_migration_test
end

protected

def create_data_migration_test
return unless DataMigrate.config.test_support_enabled

case DataMigrate::Helpers::InferTestSuiteType.new.call
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick:

... adding to my previous comment: would be nice if we could always rely on config here, instead of manually infering things each time

when :rspec
template "data_migration_spec.rb", data_migrations_spec_file_path
when :minitest
template "data_migration_test.rb", data_migrations_test_file_path
end
end

def data_migrations_test_file_path
File.join(Rails.root, 'test', data_migrations_path, "#{migration_file_name}_test.rb")
end

def data_migrations_spec_file_path
File.join(Rails.root, 'spec', data_migrations_path, "#{migration_file_name}_spec.rb")
end

def set_local_assigns!
if file_name =~ /^(add|remove)_.*_(?:to|from)_(.*)/
@migration_action = $1
Expand All @@ -38,6 +58,10 @@ def data_migrations_file_path
File.join(data_migrations_path, "#{file_name}.rb")
end

def data_migrations_file_path_with_version
File.join(data_migrations_path, "#{migration_number}_#{migration_file_name}.rb")
end

# Use the first path in the data_migrations_path as the target directory
def data_migrations_path
Array.wrap(DataMigrate.config.data_migrations_path).first
Expand Down
10 changes: 10 additions & 0 deletions lib/generators/data_migration/templates/data_migration_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require 'rails_helper'
require './<%= data_migrations_file_path_with_version %>'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vprigent Excellent idea here! Much easier and right next to the file being tested


describe <%= migration_class_name %>, type: :data_migration do
let(:migration) { <%= migration_class_name %>.new }

pending "should test `migration.up`"

pending "should test `migration.down`"
end
16 changes: 16 additions & 0 deletions lib/generators/data_migration/templates/data_migration_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require 'test_helper'
require './<%= data_migrations_file_path_with_version %>'

class <%= migration_class_name %>Test < ActiveSupport::TestCase
def setup
@migration = <%= migration_class_name %>.new
end

def test_migration_up
skip("Pending test coverage for @migration.up")
end

def test_migration_down
skip("Pending test coverage for @migration.down")
end
end
24 changes: 24 additions & 0 deletions spec/data_migrate/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,28 @@
end
end
end

describe "#test_framework=" do
context "when valid test framework is set" do
it "sets the test framework" do
allow(DataMigrate.config).to receive(:test_framework).and_return(:minitest)

expect(DataMigrate.config.test_framework).to eq(:minitest)
end
end

context "when valid test framework is set" do
it "sets the test framework" do
allow(DataMigrate.config).to receive(:test_framework).and_return(:rspec)

expect(DataMigrate.config.test_framework).to eq(:rspec)
end
end

context "when invalid test framework is set" do
it "raises an error" do
expect { DataMigrate.config.test_framework = :invalid }.to raise_error(ArgumentError, "Invalid test framework: invalid")
end
end
end
end
58 changes: 58 additions & 0 deletions spec/data_migrate/helpers/infer_test_suite_type_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require 'spec_helper'

describe DataMigrate::Helpers::InferTestSuiteType do
subject(:infer_test_suite) { described_class.new }

describe '#call' do
before do
allow(Rails).to receive(:root).and_return(Pathname.new('/fake/path'))
end

context 'when test framework is explicitly set' do
before do
allow(DataMigrate.config).to receive(:test_framework).and_return(:rspec)

# Set minitest inference to ensure :rspec if properly set
allow(File).to receive(:exist?).with(Rails.root.join('spec', 'spec_helper.rb')).and_return(false)
allow(File).to receive(:exist?).with(Rails.root.join('test', 'test_helper.rb')).and_return(true)
end

it 'returns the explicitly set test framework' do
expect(infer_test_suite.call).to eq(:rspec)
end
end

context 'when RSpec is detected' do
before do
allow(File).to receive(:exist?).with(Rails.root.join('spec', 'spec_helper.rb')).and_return(true)
allow(File).to receive(:exist?).with(Rails.root.join('test', 'test_helper.rb')).and_return(false)
end

it 'returns :rspec' do
expect(infer_test_suite.call).to eq(:rspec)
end
end

context 'when Minitest is detected' do
before do
allow(File).to receive(:exist?).with(Rails.root.join('spec', 'spec_helper.rb')).and_return(false)
allow(File).to receive(:exist?).with(Rails.root.join('test', 'test_helper.rb')).and_return(true)
end

it 'returns :minitest' do
expect(infer_test_suite.call).to eq(:minitest)
end
end

context 'when no test suite is detected' do
before do
allow(File).to receive(:exist?).with(Rails.root.join('spec', 'spec_helper.rb')).and_return(false)
allow(File).to receive(:exist?).with(Rails.root.join('test', 'test_helper.rb')).and_return(false)
end

it 'raises an error' do
expect { infer_test_suite.call }.to raise_error(StandardError, 'Unable to determine test suite')
end
end
end
end
52 changes: 50 additions & 2 deletions spec/generators/data_migration/data_migration_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
end

describe :create_data_migration do
subject { DataMigrate::Generators::DataMigrationGenerator.new(['my_migration']) }
let(:migration_name) { 'my_migration' }

let(:data_migrations_file_path) { 'abc/my_migration.rb' }
subject { DataMigrate::Generators::DataMigrationGenerator.new([migration_name]) }

let(:data_migrations_file_path) { "abc/#{migration_name}.rb" }

context 'when custom data migrations path has a trailing slash' do
before do
Expand All @@ -44,6 +46,8 @@
'data_migration.rb', data_migrations_file_path
)

expect(subject).not_to receive(:template)

subject.create_data_migration
end
end
Expand All @@ -58,9 +62,53 @@
'data_migration.rb', data_migrations_file_path
)

expect(subject).not_to receive(:template)

subject.create_data_migration
end
end

context 'when test support is enabled' do
let(:rails_root) { 'dummy-app' }

before do
DataMigrate.config.data_migrations_path = 'db/data/'
DataMigrate.config.test_support_enabled = true

allow(Rails).to receive(:root).and_return(Pathname.new(rails_root))
expect(DataMigrate::Helpers::InferTestSuiteType).to receive(:new).and_return(infer_double)
end

after(:all) do
FileUtils.rm_rf(Dir.glob("#{DataMigrate.config.data_migrations_path}/*"))
end

context 'and the test suite is RSpec' do
let(:infer_double) { instance_double(DataMigrate::Helpers::InferTestSuiteType, call: :rspec) }

it 'creates a spec file' do
expect(subject).to receive(:template).with(
'data_migration_spec.rb',
"#{rails_root}/spec/db/data/#{migration_name}_spec.rb"
)

subject.create_data_migration
end
end

context 'and the test suite is Minitest' do
let(:infer_double) { instance_double(DataMigrate::Helpers::InferTestSuiteType, call: :minitest) }

it 'creates a test file' do
expect(subject).to receive(:template).with(
'data_migration_test.rb',
"#{rails_root}/test/db/data/#{migration_name}_test.rb"
)

subject.create_data_migration
end
end
end
end

describe ".source_root" do
Expand Down
Loading