Merge branch 'db-testing-catch-system-exceptions' into 'master'

Rescue low-level errors during migration testing

See merge request gitlab-org/gitlab!79445

lib/gitlab/database/migrations/instrumentation.rb

@@ -15,30 +15,26 @@ module Gitlab
         end
 
         def observe(version:, name:, connection:, &block)
-          observation = Observation.new(version, name)
-          observation.success = true
+          observation = Observation.new(version: version, name: name, success: false)
 
           observers = observer_classes.map { |c| c.new(observation, @result_dir, connection) }
 
-          exception = nil
-
           on_each_observer(observers) { |observer| observer.before }
 
-          observation.walltime = Benchmark.realtime do
-            yield
-          rescue StandardError => e
-            exception = e
-            observation.success = false
-          end
+          start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+
+          yield
+
+          observation.success = true
+
+          observation
+        ensure
+          observation.walltime = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
 
           on_each_observer(observers) { |observer| observer.after }
           on_each_observer(observers) { |observer| observer.record }
 
           record_observation(observation)
-
-          raise exception if exception
-
-          observation
         end
 
         private

lib/gitlab/database/migrations/observation.rb

@@ -10,7 +10,8 @@ module Gitlab
         :walltime,
         :success,
         :total_database_size_change,
-        :query_statistics
+        :query_statistics,
+        keyword_init: true
       )
     end
   end

spec/lib/gitlab/database/migrations/instrumentation_spec.rb

@@ -66,55 +66,43 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
     context 'on successful execution' do
       subject { described_class.new(result_dir: result_dir).observe(version: migration_version, name: migration_name, connection: connection) {} }
 
-      it 'records walltime' do
+      it 'records a valid observation', :aggregate_failures do
         expect(subject.walltime).not_to be_nil
-      end
-
-      it 'records success' do
         expect(subject.success).to be_truthy
-      end
-
-      it 'records the migration version' do
         expect(subject.version).to eq(migration_version)
-      end
-
-      it 'records the migration name' do
         expect(subject.name).to eq(migration_name)
       end
     end
 
     context 'upon failure' do
-      subject { described_class.new(result_dir: result_dir).observe(version: migration_version, name: migration_name, connection: connection) { raise 'something went wrong' } }
-
-      it 'raises the exception' do
-        expect { subject }.to raise_error(/something went wrong/)
-      end
-
-      context 'retrieving observations' do
-        subject { instance.observations.first }
-
-        before do
-          instance.observe(version: migration_version, name: migration_name, connection: connection) { raise 'something went wrong' }
-        rescue StandardError
-          # ignore
-        end
+      where(exception: ['something went wrong', SystemStackError, Interrupt])
 
+      with_them do
         let(:instance) { described_class.new(result_dir: result_dir) }
 
-        it 'records walltime' do
-          expect(subject.walltime).not_to be_nil
-        end
-
-        it 'records failure' do
-          expect(subject.success).to be_falsey
-        end
+        subject(:observe) { instance.observe(version: migration_version, name: migration_name, connection: connection) { raise exception } }
 
-        it 'records the migration version' do
-          expect(subject.version).to eq(migration_version)
+        it 'raises the exception' do
+          expect { observe }.to raise_error(exception)
         end
 
-        it 'records the migration name' do
-          expect(subject.name).to eq(migration_name)
+        context 'retrieving observations' do
+          subject { instance.observations.first }
+
+          before do
+            observe
+            # rubocop:disable Lint/RescueException
+          rescue Exception
+            # rubocop:enable Lint/RescueException
+            # ignore (we expect this exception)
+          end
+
+          it 'records a valid observation', :aggregate_failures do
+            expect(subject.walltime).not_to be_nil
+            expect(subject.success).to be_falsey
+            expect(subject.version).to eq(migration_version)
+            expect(subject.name).to eq(migration_name)
+          end
         end
       end
     end

spec/lib/gitlab/database/migrations/observers/query_details_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do
   subject { described_class.new(observation, directory_path, connection) }
 
   let(:connection) { ActiveRecord::Migration.connection }
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(version: migration_version, name: migration_name) }
   let(:query) { "select date_trunc('day', $1::timestamptz) + $2 * (interval '1 hour')" }
   let(:query_binds) { [Time.current, 3] }
   let(:directory_path) { Dir.mktmpdir }

spec/lib/gitlab/database/migrations/observers/query_log_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
   subject { described_class.new(observation, directory_path, connection) }
 
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(version: migration_version, name: migration_name) }
   let(:connection) { ActiveRecord::Migration.connection }
   let(:query) { 'select 1' }
   let(:directory_path) { Dir.mktmpdir }

spec/lib/gitlab/database/migrations/observers/transaction_duration_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::TransactionDuration do
   subject(:transaction_duration_observer) { described_class.new(observation, directory_path, connection) }
 
   let(:connection) { ActiveRecord::Migration.connection }
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(version: migration_version, name: migration_name) }
   let(:directory_path) { Dir.mktmpdir }
   let(:log_file) { "#{directory_path}/#{migration_version}_#{migration_name}-transaction-duration.json" }
   let(:transaction_duration) { Gitlab::Json.parse(File.read(log_file)) }