From 71a90dcba20e97c30f3131f6dadbc2ae661348a9 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Sun, 30 Mar 2025 22:36:50 +0200 Subject: [PATCH] DEV: Refactor migrations-tooling * Updates GitHub Action for migrations * Rubocop: Always `EnforcedShorthandSyntax` for hashes in the `migrations` directory * Automatically load all available converter steps * Enable YJIT at runtime, if available * Progressbar shows skipped records and other small improvements --- .github/workflows/migration-tests.yml | 39 ++--- migrations/.rubocop.yml | 4 + migrations/bin/cli | 8 +- migrations/config/locales/migrations.en.yml | 3 + migrations/lib/common/date_helper.rb | 23 ++- .../lib/common/extended_progress_bar.rb | 77 +++++----- migrations/lib/converters/base/converter.rb | 11 +- .../converters/base/progress_step_executor.rb | 12 +- .../lib/converters/base/step_tracker.rb | 7 +- .../lib/converters/example/steps/step1.rb | 2 +- .../lib/converters/example/steps/step2.rb | 2 +- .../lib/converters/example/steps/step3.rb | 2 +- migrations/lib/database.rb | 12 ++ migrations/lib/database/connection.rb | 37 +++-- .../lib/database/intermediate_db/log_entry.rb | 2 +- .../lib/database/intermediate_db/upload.rb | 133 +++++++++--------- migrations/lib/database/migrator.rb | 4 +- migrations/lib/uploader/tasks/fixer.rb | 2 +- migrations/{lib => }/migrations.rb | 12 +- migrations/scripts/benchmarks/hash_vs_data.rb | 2 +- .../scripts/benchmarks/parameter_binding.rb | 2 +- .../lib/converters/base/step_tracker_spec.rb | 22 +-- .../spec/lib/database/intermediate_db_spec.rb | 21 +++ migrations/spec/lib/database/migrator_spec.rb | 10 +- migrations/spec/rails_helper.rb | 2 +- migrations/spec/support/helpers.rb | 4 + .../shared_examples/database_entity.rb | 4 +- 27 files changed, 258 insertions(+), 201 deletions(-) create mode 100644 migrations/.rubocop.yml rename migrations/{lib => }/migrations.rb (87%) diff --git a/.github/workflows/migration-tests.yml b/.github/workflows/migration-tests.yml index a1f4e119f8c..7bcd4443133 100644 --- a/.github/workflows/migration-tests.yml +++ b/.github/workflows/migration-tests.yml @@ -23,21 +23,16 @@ permissions: jobs: tests: if: github.event_name == 'pull_request' || github.repository != 'discourse/discourse-private-mirror' - name: Tests with Ruby ${{ matrix.ruby }} - runs-on: ${{ (github.repository != 'discourse/discourse' && 'ubuntu-latest') || 'debian-12' }} - container: discourse/discourse_test:slim + name: Tests + runs-on: ${{ (github.repository_owner == 'discourse' && 'debian-12') || 'ubuntu-latest' }} + container: discourse/discourse_test:release timeout-minutes: 20 env: RAILS_ENV: test PGUSER: discourse PGPASSWORD: discourse - - strategy: - fail-fast: false - - matrix: - ruby: ["3.3"] + LOAD_PLUGINS: 1 steps: - name: Set working directory owner @@ -62,23 +57,9 @@ jobs: sudo -E -u postgres script/start_test_db.rb sudo -u postgres psql -c "CREATE ROLE $PGUSER LOGIN SUPERUSER PASSWORD '$PGPASSWORD';" - - name: Container envs - id: container-envs + - name: Symlink vendor/bundle from image run: | - echo "ruby_version=$RUBY_VERSION" >> $GITHUB_OUTPUT - echo "debian_release=$DEBIAN_RELEASE" >> $GITHUB_OUTPUT - shell: bash - - - name: Bundler cache - uses: actions/cache@v4 - with: - path: vendor/bundle - key: >- - ${{ runner.os }}- - ${{ steps.container-envs.outputs.ruby_version }}- - ${{ steps.container-envs.outputs.debian_release }}- - ${{ hashFiles('**/Gemfile.lock') }}- - migrations-tooling + ln -s /var/www/discourse/vendor/bundle vendor/bundle - name: Setup gems run: | @@ -88,11 +69,14 @@ jobs: bundle config --local without development bundle config --local with migrations bundle install --jobs $(($(nproc) - 1)) - bundle clean - name: pnpm install run: pnpm install --frozen-lockfile + - name: Get CPU cores + id: cpu-info + run: echo "cpu-cores=$(nproc)" >> $GITHUB_OUTPUT + - name: Fetch app state cache uses: actions/cache@v4 id: app-cache @@ -100,7 +84,8 @@ jobs: path: tmp/app-cache key: >- ${{ runner.os }}- - ${{ hashFiles('.github/workflows/tests.yml') }}- + ${{ steps.cpu-info.outputs.cpu-cores }}- + ${{ hashFiles('.github/workflows/migration-tests.yml') }}- ${{ hashFiles('db/**/*', 'plugins/**/db/**/*') }}- ${{ hashFiles('config/environments/test.rb') }} diff --git a/migrations/.rubocop.yml b/migrations/.rubocop.yml new file mode 100644 index 00000000000..aaa23c567d7 --- /dev/null +++ b/migrations/.rubocop.yml @@ -0,0 +1,4 @@ +inherit_from: ../.rubocop.yml + +Style/HashSyntax: + EnforcedShorthandSyntax: always diff --git a/migrations/bin/cli b/migrations/bin/cli index 2341a0caa25..e0894181f94 100755 --- a/migrations/bin/cli +++ b/migrations/bin/cli @@ -1,7 +1,7 @@ #!/usr/bin/env ruby # frozen_string_literal: true -require_relative "../lib/migrations" +require_relative "../migrations" require "colored2" require "thor" @@ -44,6 +44,12 @@ module Migrations end end +if defined?(RubyVM::YJIT) + RubyVM::YJIT.enable +else + warn "WARNING: Performance degraded: RubyVM::YJIT is not available".yellow +end + # rubocop:disable Discourse/NoChdir Dir.chdir(File.expand_path("../..", __dir__)) { ::Migrations::CLI::Application.start } # rubocop:enable Discourse/NoChdir diff --git a/migrations/config/locales/migrations.en.yml b/migrations/config/locales/migrations.en.yml index e8f7fd2058e..c2c9168ba10 100644 --- a/migrations/config/locales/migrations.en.yml +++ b/migrations/config/locales/migrations.en.yml @@ -1,5 +1,8 @@ en: progressbar: + skips: + one: "%{count} skip" + other: "%{count} skips" warnings: one: "%{count} warning" other: "%{count} warnings" diff --git a/migrations/lib/common/date_helper.rb b/migrations/lib/common/date_helper.rb index 9cf8fdc8fd3..8858c35f2e2 100644 --- a/migrations/lib/common/date_helper.rb +++ b/migrations/lib/common/date_helper.rb @@ -2,21 +2,16 @@ module Migrations module DateHelper - # based on code from https://gist.github.com/emmahsax/af285a4b71d8506a1625a3e591dc993b - def self.human_readable_time(secs) - return "< 1 second" if secs < 1 + def self.human_readable_time(seconds) + hours, remainder = seconds.divmod(3600) + minutes, seconds = remainder.divmod(60) + format("%02d:%02d:%02d", hours, minutes, seconds) + end - [[60, :seconds], [60, :minutes], [24, :hours], [Float::INFINITY, :days]].map do |count, name| - next if secs <= 0 - - secs, number = secs.divmod(count) - unless number.to_i == 0 - "#{number.to_i} #{number == 1 ? name.to_s.delete_suffix("s") : name}" - end - end - .compact - .reverse - .join(", ") + def self.track_time + start_time = Time.now + yield + Time.now - start_time end end end diff --git a/migrations/lib/common/extended_progress_bar.rb b/migrations/lib/common/extended_progress_bar.rb index 7583253e467..b9ed0d101e1 100644 --- a/migrations/lib/common/extended_progress_bar.rb +++ b/migrations/lib/common/extended_progress_bar.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "colored2" require "ruby-progressbar" module Migrations @@ -7,9 +8,10 @@ module Migrations def initialize(max_progress: nil) @max_progress = max_progress + @skip_count = 0 @warning_count = 0 @error_count = 0 - @extra_information = "" + @extra_information = +"" @base_format = nil @progressbar = nil @@ -18,60 +20,58 @@ module Migrations def run raise "ProgressBar already started" if @progressbar - format = setup_progressbar + format = calculate_format + setup_progressbar + yield self + finalize_progressbar(format) nil end - def update(progress, warning_count, error_count) - extra_information_changed = false + def update(increment_by:, skip_count: 0, warning_count: 0, error_count: 0) + updated = false + + if skip_count > 0 + @skip_count += skip_count + updated = true + end if warning_count > 0 @warning_count += warning_count - extra_information_changed = true + updated = true end if error_count > 0 @error_count += error_count - extra_information_changed = true + updated = true end - if extra_information_changed - @extra_information = +"" + update_format if updated - if @warning_count > 0 - @extra_information << " | " << - I18n.t("progressbar.warnings", count: @warning_count).yellow - end - - if @error_count > 0 - @extra_information << " | " << I18n.t("progressbar.errors", count: @error_count).red - end - - @progressbar.format = "#{@base_format}#{@extra_information}" - end - - if progress == 1 + if increment_by == 1 @progressbar.increment else - @progressbar.progress += progress + @progressbar.progress += increment_by end end private + def calculate_format + if @max_progress + format = I18n.t("progressbar.processed.progress_with_max", current: "%c", max: "%C") + @base_format = " %a | %E | #{format}" + else + format = I18n.t("progressbar.processed.progress", current: "%c") + @base_format = " %a | #{format}" + end + + format + end + def setup_progressbar - format = - if @max_progress - I18n.t("progressbar.processed.progress_with_max", current: "%c", max: "%C") - else - I18n.t("progressbar.processed.progress", current: "%c") - end - - @base_format = @max_progress ? " %a |%E | #{format}" : " %a | #{format}" - @progressbar = ::ProgressBar.create( total: @max_progress, @@ -83,14 +83,23 @@ module Migrations format: @base_format, throttle_rate: 0.5, ) + end - format + def update_format + @extra_information.clear + + messages = [] + messages << I18n.t("progressbar.skips", count: @skip_count).cyan if @skip_count > 0 + messages << I18n.t("progressbar.warnings", count: @warning_count).yellow if @warning_count > 0 + messages << I18n.t("progressbar.errors", count: @error_count).red if @error_count > 0 + + @extra_information << " | #{messages.join(" | ")}" unless messages.empty? + @progressbar.format = "#{@base_format}#{@extra_information}" end def finalize_progressbar(format) print "\033[K" # delete the output of progressbar, because it doesn't overwrite longer lines - final_format = @max_progress ? " %a | #{format}" : " %a | #{format}" - @progressbar.format = "#{final_format}#{@extra_information}" + @progressbar.format = " %a | #{format}#{@extra_information}" @progressbar.finish end end diff --git a/migrations/lib/converters/base/converter.rb b/migrations/lib/converters/base/converter.rb index e7847ca6b3f..211d4c86204 100644 --- a/migrations/lib/converters/base/converter.rb +++ b/migrations/lib/converters/base/converter.rb @@ -30,7 +30,14 @@ module Migrations::Converters::Base end def steps - raise NotImplementedError + step_class = ::Migrations::Converters::Base::Step + current_module = self.class.name.deconstantize.constantize + + current_module + .constants + .map { |c| current_module.const_get(c) } + .select { |klass| klass.is_a?(Class) && klass < step_class } + .sort_by(&:to_s) end def before_step_execution(step) @@ -70,7 +77,7 @@ module Migrations::Converters::Base end def create_step(step_class) - default_args = { settings: settings } + default_args = { settings: } args = default_args.merge(step_args(step_class)) step_class.new(StepTracker.new, args) diff --git a/migrations/lib/converters/base/progress_step_executor.rb b/migrations/lib/converters/base/progress_step_executor.rb index d9aace10f64..24afe210176 100644 --- a/migrations/lib/converters/base/progress_step_executor.rb +++ b/migrations/lib/converters/base/progress_step_executor.rb @@ -39,7 +39,11 @@ module Migrations::Converters::Base with_progressbar do |progressbar| @step.items.each do |item| stats = job.run(item) - progressbar.update(stats.progress, stats.warning_count, stats.error_count) + progressbar.update( + increment_by: stats.progress, + warning_count: stats.warning_count, + error_count: stats.error_count, + ) end end end @@ -90,7 +94,11 @@ module Migrations::Converters::Base ::Migrations::Database::IntermediateDB.insert(sql, *parameters) end - progressbar.update(stats.progress, stats.warning_count, stats.error_count) + progressbar.update( + increment_by: stats.progress, + warning_count: stats.warning_count, + error_count: stats.error_count, + ) end end end diff --git a/migrations/lib/converters/base/step_tracker.rb b/migrations/lib/converters/base/step_tracker.rb index 4e865042755..7c16d7d170a 100644 --- a/migrations/lib/converters/base/step_tracker.rb +++ b/migrations/lib/converters/base/step_tracker.rb @@ -36,12 +36,7 @@ module Migrations::Converters::Base private def log(type, message, exception: nil, details: nil) - ::Migrations::Database::IntermediateDB::LogEntry.create!( - type:, - message:, - exception:, - details:, - ) + ::Migrations::Database::IntermediateDB::LogEntry.create(type:, message:, exception:, details:) end end end diff --git a/migrations/lib/converters/example/steps/step1.rb b/migrations/lib/converters/example/steps/step1.rb index e7cda29538a..85eff6674b0 100644 --- a/migrations/lib/converters/example/steps/step1.rb +++ b/migrations/lib/converters/example/steps/step1.rb @@ -6,7 +6,7 @@ module Migrations::Converters::Example def execute super - IntermediateDB::LogEntry.create!(type: "info", message: "This is a test") + IntermediateDB::LogEntry.create(type: "info", message: "This is a test") end end end diff --git a/migrations/lib/converters/example/steps/step2.rb b/migrations/lib/converters/example/steps/step2.rb index d423222e3ef..26a230ed253 100644 --- a/migrations/lib/converters/example/steps/step2.rb +++ b/migrations/lib/converters/example/steps/step2.rb @@ -14,7 +14,7 @@ module Migrations::Converters::Example step.log_warning("Test", details: item) if item.in?([3, 7, 9]) step.log_error("Test", details: item) if item.in?([6, 10]) - IntermediateDB::LogEntry.create!(type: "info", message: "Step2 - #{item}") + IntermediateDB::LogEntry.create(type: "info", message: "Step2 - #{item}") end end end diff --git a/migrations/lib/converters/example/steps/step3.rb b/migrations/lib/converters/example/steps/step3.rb index 8c961192b7b..b00f305001b 100644 --- a/migrations/lib/converters/example/steps/step3.rb +++ b/migrations/lib/converters/example/steps/step3.rb @@ -17,7 +17,7 @@ module Migrations::Converters::Example step.log_warning("Test", details: item) if item[:counter] > 10 && item[:counter] < 20 - IntermediateDB::LogEntry.create!(type: "info", message: "Step3 - #{item[:counter]}") + IntermediateDB::LogEntry.create(type: "info", message: "Step3 - #{item[:counter]}") end end end diff --git a/migrations/lib/database.rb b/migrations/lib/database.rb index 17665e33e96..fac8428eeed 100644 --- a/migrations/lib/database.rb +++ b/migrations/lib/database.rb @@ -59,5 +59,17 @@ module Migrations return nil if value.nil? ::Oj.dump(value, mode: :compat) end + + def self.to_date(text) + text.present? ? Date.parse(text) : nil + end + + def self.to_datetime(text) + text.present? ? DateTime.parse(text) : nil + end + + def self.to_boolean(value) + value == 1 + end end end diff --git a/migrations/lib/database/connection.rb b/migrations/lib/database/connection.rb index 189d3f26b4c..913cceb1654 100644 --- a/migrations/lib/database/connection.rb +++ b/migrations/lib/database/connection.rb @@ -8,6 +8,7 @@ module Migrations::Database PREPARED_STATEMENT_CACHE_SIZE = 5 def self.open_database(path:) + path = File.expand_path(path, ::Migrations.root_path) FileUtils.mkdir_p(File.dirname(path)) db = ::Extralite::Database.new(path) @@ -25,7 +26,7 @@ module Migrations::Database attr_reader :db, :path def initialize(path:, transaction_batch_size: TRANSACTION_BATCH_SIZE) - @path = path + @path = File.expand_path(path, ::Migrations.root_path) @transaction_batch_size = transaction_batch_size @db = self.class.open_database(path:) @statement_counter = 0 @@ -54,24 +55,34 @@ module Migrations::Database if (@statement_counter += 1) >= @transaction_batch_size commit_transaction + end + end + + def query(sql, *parameters, &block) + @db.query(sql, *parameters, &block) + end + + def count(sql, *parameters) + @db.query_single_splat(sql, *parameters) + end + + def execute(sql, *parameters) + @db.execute(sql, *parameters) + end + + def begin_transaction + @db.execute("BEGIN DEFERRED TRANSACTION") unless @db.transaction_active? + end + + def commit_transaction + if @db.transaction_active? + @db.execute("COMMIT") @statement_counter = 0 end end private - def begin_transaction - return if @db.transaction_active? - - @db.execute("BEGIN DEFERRED TRANSACTION") - end - - def commit_transaction - return unless @db.transaction_active? - - @db.execute("COMMIT") - end - def close_connection(keep_path:) return if @db.nil? diff --git a/migrations/lib/database/intermediate_db/log_entry.rb b/migrations/lib/database/intermediate_db/log_entry.rb index 00f568d8e8e..d2d93535517 100644 --- a/migrations/lib/database/intermediate_db/log_entry.rb +++ b/migrations/lib/database/intermediate_db/log_entry.rb @@ -11,7 +11,7 @@ module Migrations::Database::IntermediateDB VALUES (?, ?, ?, ?, ?) SQL - def self.create!(created_at: Time.now, type:, message:, exception: nil, details: nil) + def self.create(created_at: Time.now, type:, message:, exception: nil, details: nil) ::Migrations::Database::IntermediateDB.insert( SQL, ::Migrations::Database.format_datetime(created_at), diff --git a/migrations/lib/database/intermediate_db/upload.rb b/migrations/lib/database/intermediate_db/upload.rb index 4da5aee8db8..59d4a7cd294 100644 --- a/migrations/lib/database/intermediate_db/upload.rb +++ b/migrations/lib/database/intermediate_db/upload.rb @@ -17,76 +17,79 @@ module Migrations::Database::IntermediateDB VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) SQL - class << self - def create_for_file!( + def self.create_for_file( + path:, + filename: nil, + type: nil, + description: nil, + origin: nil, + user_id: nil + ) + create( + id: ::Migrations::ID.hash(path), + filename: filename || File.basename(path), path:, - filename: nil, - type: nil, - description: nil, - origin: nil, - user_id: nil + type:, + description:, + origin:, + user_id:, ) - create!( - id: ::Migrations::ID.hash(path), - filename: filename || File.basename(path), - path:, - type:, - description:, - origin:, - user_id:, - ) - end + end - def create_for_url!(url:, filename:, type: nil, description: nil, origin: nil, user_id: nil) - create!( - id: ::Migrations::ID.hash(url), - filename:, - url:, - type:, - description:, - origin:, - user_id:, - ) - end - - def create_for_data!(data:, filename:, type: nil, description: nil, origin: nil, user_id: nil) - create!( - id: ::Migrations::ID.hash(data), - filename:, - data: ::Migrations::Database.to_blob(data), - type:, - description:, - origin:, - user_id:, - ) - end - - private - - def create!( - id:, + def self.create_for_url(url:, filename:, type: nil, description: nil, origin: nil, user_id: nil) + create( + id: ::Migrations::ID.hash(url), filename:, - path: nil, - data: nil, - url: nil, - type: nil, - description: nil, - origin: nil, - user_id: nil + url:, + type:, + description:, + origin:, + user_id:, + ) + end + + def self.create_for_data( + data:, + filename:, + type: nil, + description: nil, + origin: nil, + user_id: nil + ) + create( + id: ::Migrations::ID.hash(data), + filename:, + data: ::Migrations::Database.to_blob(data), + type:, + description:, + origin:, + user_id:, + ) + end + + def self.create( + id:, + filename:, + path: nil, + data: nil, + url: nil, + type: nil, + description: nil, + origin: nil, + user_id: nil + ) + ::Migrations::Database::IntermediateDB.insert( + SQL, + id, + filename, + path, + data, + url, + type, + description, + origin, + user_id, ) - ::Migrations::Database::IntermediateDB.insert( - SQL, - id, - filename, - path, - data, - url, - type, - description, - origin, - user_id, - ) - end end end end diff --git a/migrations/lib/database/migrator.rb b/migrations/lib/database/migrator.rb index 4ede9e5be53..2316a7d75dc 100644 --- a/migrations/lib/database/migrator.rb +++ b/migrations/lib/database/migrator.rb @@ -3,7 +3,7 @@ module Migrations::Database class Migrator def initialize(db_path) - @db_path = db_path + @db_path = File.expand_path(db_path, ::Migrations.root_path) @db = nil end @@ -72,7 +72,7 @@ module Migrations::Database @db.transaction do @db.execute(sql) - @db.execute(<<~SQL, path: relative_path, sql_hash: sql_hash) + @db.execute(<<~SQL, path: relative_path, sql_hash:) INSERT INTO schema_migrations (path, created_at, sql_hash) VALUES (:path, datetime('now'), :sql_hash) SQL diff --git a/migrations/lib/uploader/tasks/fixer.rb b/migrations/lib/uploader/tasks/fixer.rb index 20b2bee09e9..964051909ff 100644 --- a/migrations/lib/uploader/tasks/fixer.rb +++ b/migrations/lib/uploader/tasks/fixer.rb @@ -71,7 +71,7 @@ module Migrations::Uploader end def update_status_queue(row, upload, status) - status_queue << { id: row[:id], upload_id: upload[:id], status: status } + status_queue << { id: row[:id], upload_id: upload[:id], status: } end def log_status diff --git a/migrations/lib/migrations.rb b/migrations/migrations.rb similarity index 87% rename from migrations/lib/migrations.rb rename to migrations/migrations.rb index eb67a56da45..16341d2a6c5 100644 --- a/migrations/lib/migrations.rb +++ b/migrations/migrations.rb @@ -7,21 +7,21 @@ require "active_support" require "active_support/core_ext" require "zeitwerk" -require_relative "converters" +require_relative "lib/converters" module Migrations class NoSettingsFound < StandardError end def self.root_path - @root_path ||= File.expand_path("..", __dir__) + @root_path ||= __dir__ end def self.load_rails_environment(quiet: false) - message = "Loading Rails environment ..." + message = "Loading Rails environment..." print message if !quiet - rails_root = File.expand_path("../..", __dir__) + rails_root = File.expand_path("..", __dir__) # rubocop:disable Discourse/NoChdir Dir.chdir(rails_root) do begin @@ -48,7 +48,9 @@ module Migrations { "cli" => "CLI", "id" => "ID", + "discourse_db" => "DiscourseDB", "intermediate_db" => "IntermediateDB", + "mappings_db" => "MappingsDB", "uploads_db" => "UploadsDB", }, ) @@ -64,7 +66,7 @@ module Migrations Dir[File.join(converter_path, "**", "*")].each do |subdirectory| next unless File.directory?(subdirectory) - loader.push_dir(subdirectory, namespace: namespace) + loader.push_dir(subdirectory, namespace:) end end diff --git a/migrations/scripts/benchmarks/hash_vs_data.rb b/migrations/scripts/benchmarks/hash_vs_data.rb index 20bb209bcc7..6f0788853bd 100755 --- a/migrations/scripts/benchmarks/hash_vs_data.rb +++ b/migrations/scripts/benchmarks/hash_vs_data.rb @@ -20,7 +20,7 @@ User = Data.define(:id, :name, :email, :created_at) USER_HASH = begin name = SecureRandom.hex(10) - { id: 1, name: name, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 } + { id: 1, name:, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 } end USER_DATA = diff --git a/migrations/scripts/benchmarks/parameter_binding.rb b/migrations/scripts/benchmarks/parameter_binding.rb index a3cc17b4d97..56de1b1aa16 100755 --- a/migrations/scripts/benchmarks/parameter_binding.rb +++ b/migrations/scripts/benchmarks/parameter_binding.rb @@ -41,7 +41,7 @@ end def create_users(row_count) row_count.times.map do |id| name = SecureRandom.hex(10) - { id: id, name: name, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 } + { id:, name:, email: "#{name}@example.com", created_at: Time.now.utc.iso8601 } end end diff --git a/migrations/spec/lib/converters/base/step_tracker_spec.rb b/migrations/spec/lib/converters/base/step_tracker_spec.rb index db8d2fd91b8..4af9b0d8a03 100644 --- a/migrations/spec/lib/converters/base/step_tracker_spec.rb +++ b/migrations/spec/lib/converters/base/step_tracker_spec.rb @@ -3,7 +3,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do subject(:tracker) { described_class.new } - before { allow(::Migrations::Database::IntermediateDB::LogEntry).to receive(:create!) } + before { allow(::Migrations::Database::IntermediateDB::LogEntry).to receive(:create) } describe "#initialize" do it "starts at the correct values" do @@ -75,7 +75,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do it "logs an info message" do tracker.log_info("Info message") - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::INFO, message: "Info message", exception: nil, @@ -86,7 +86,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do it "logs an info message with details" do tracker.log_info("Info message", details: { key: "value" }) - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::INFO, message: "Info message", exception: nil, @@ -103,7 +103,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do tracker.stats.warning_count }.by(1) - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::WARNING, message: "Warning message", exception: nil, @@ -115,13 +115,13 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do exception = StandardError.new("Warning exception") expect { - tracker.log_warning("Warning message", exception: exception, details: { key: "value" }) + tracker.log_warning("Warning message", exception:, details: { key: "value" }) }.to change { tracker.stats.warning_count }.by(1) - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::WARNING, message: "Warning message", - exception: exception, + exception:, details: { key: "value", }, @@ -133,7 +133,7 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do it "logs an error message and increments error_count" do expect { tracker.log_error("Error message") }.to change { tracker.stats.error_count }.by(1) - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::ERROR, message: "Error message", exception: nil, @@ -145,13 +145,13 @@ RSpec.describe ::Migrations::Converters::Base::StepTracker do exception = StandardError.new("Error exception") expect { - tracker.log_error("Error message", exception: exception, details: { key: "value" }) + tracker.log_error("Error message", exception:, details: { key: "value" }) }.to change { tracker.stats.error_count }.by(1) - expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create!).with( + expect(::Migrations::Database::IntermediateDB::LogEntry).to have_received(:create).with( type: ::Migrations::Database::IntermediateDB::LogEntry::ERROR, message: "Error message", - exception: exception, + exception:, details: { key: "value", }, diff --git a/migrations/spec/lib/database/intermediate_db_spec.rb b/migrations/spec/lib/database/intermediate_db_spec.rb index fd429eef9be..29453218a4b 100644 --- a/migrations/spec/lib/database/intermediate_db_spec.rb +++ b/migrations/spec/lib/database/intermediate_db_spec.rb @@ -94,4 +94,25 @@ RSpec.describe ::Migrations::Database::IntermediateDB do end end end + + it "checks that manually created entities are tested" do + Dir[ + File.join(::Migrations.root_path, "lib", "database", "intermediate_db", "*.rb") + ].each do |path| + next if File.read(path).include?("auto-generated") + + spec_path = + File.join( + ::Migrations.root_path, + "spec", + "lib", + "database", + "intermediate_db", + "#{File.basename(path, ".rb")}_spec.rb", + ) + + expect(File.exist?(spec_path)).to eq(true), "#{spec_path} is missing" + expect(File.read(spec_path)).to include('it_behaves_like "a database entity"') + end + end end diff --git a/migrations/spec/lib/database/migrator_spec.rb b/migrations/spec/lib/database/migrator_spec.rb index 04985f25199..aa5fe97cc46 100644 --- a/migrations/spec/lib/database/migrator_spec.rb +++ b/migrations/spec/lib/database/migrator_spec.rb @@ -9,15 +9,7 @@ RSpec.describe ::Migrations::Database::Migrator do ignore_errors: false ) if migrations_directory - migrations_path = - File.join( - ::Migrations.root_path, - "spec", - "support", - "fixtures", - "schema", - migrations_directory, - ) + migrations_path = File.join(fixture_root, "schema", migrations_directory) end temp_path = storage_path = Dir.mktmpdir if storage_path.nil? diff --git a/migrations/spec/rails_helper.rb b/migrations/spec/rails_helper.rb index f1d6b987b46..b9ff87d859a 100644 --- a/migrations/spec/rails_helper.rb +++ b/migrations/spec/rails_helper.rb @@ -3,7 +3,7 @@ # we need to require the rails_helper from core to load the Rails environment require_relative "../../spec/rails_helper" -require_relative "../lib/migrations" +require_relative "../migrations" ::Migrations.configure_zeitwerk ::Migrations.enable_i18n diff --git a/migrations/spec/support/helpers.rb b/migrations/spec/support/helpers.rb index dfb1d6c4d98..12e19f43122 100644 --- a/migrations/spec/support/helpers.rb +++ b/migrations/spec/support/helpers.rb @@ -5,3 +5,7 @@ def reset_memoization(instance, *variables) instance.remove_instance_variable(var) if instance.instance_variable_defined?(var) end end + +def fixture_root + @fixture_root ||= File.join(::Migrations.root_path, "spec", "support", "fixtures") +end diff --git a/migrations/spec/support/shared_examples/database_entity.rb b/migrations/spec/support/shared_examples/database_entity.rb index c0d3f053484..e9a5a8f682a 100644 --- a/migrations/spec/support/shared_examples/database_entity.rb +++ b/migrations/spec/support/shared_examples/database_entity.rb @@ -5,7 +5,7 @@ RSpec.shared_examples "a database entity" do expect(subject).to have_constant(:SQL) end - it "responds to .create!" do - expect(subject).to respond_to(:create!) + it "responds to .create" do + expect(subject).to respond_to(:create) end end