From 7af242029e4c111510c37bf32911e3749414a51c Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 12 Dec 2024 15:43:57 -0500 Subject: [PATCH] Allow the rails method to be called, don't expect it In rails 7.0, it's called 10 times plus 1 more on the next line. In rails 7.1+, it's called only once. From a test perspective, we don't care if it's even called. We only care that the SQL query is executed with the scramsha password or not. To change the username/password only in calls from the migration itself and not from rails, we hack around the caller_locations to make the change only for the migration callers. With this, I believe we can expect test success on rails 7.1 and 7.2. It was added in 742841cf8954be1cb5843dca33945b5fc001cf8d but I think this change emphasizes what we really care about testing and removes an assertion on internals that we don't really have interest in testing. Fixes #768 --- .github/workflows/ci.yaml | 1 - ...0241017013023_reencrypt_password_scramsha_spec.rb | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 951e0b7b..2fbd5e8f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,7 +48,6 @@ jobs: run: bin/setup - name: Run tests run: bundle exec rake - continue-on-error: ${{ matrix.rails-version == '7.2' || matrix.rails-version == '7.1'}} - name: Report code coverage if: ${{ github.ref == 'refs/heads/master' && matrix.ruby-version == '3.3' && matrix.rails-version == '7.0' }} continue-on-error: true diff --git a/spec/migrations/20241017013023_reencrypt_password_scramsha_spec.rb b/spec/migrations/20241017013023_reencrypt_password_scramsha_spec.rb index 5bacb2f7..eac7c2db 100644 --- a/spec/migrations/20241017013023_reencrypt_password_scramsha_spec.rb +++ b/spec/migrations/20241017013023_reencrypt_password_scramsha_spec.rb @@ -9,9 +9,9 @@ username = ActiveRecord::Base.connection_db_config.configuration_hash[:username] - expect(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).exactly(10).times.and_call_original - expect(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).and_wrap_original do |original_method, *args, &block| - original_method.call(*args, &block).dup.tap { |i| i[:password] ||= "abc" } + allow(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).and_wrap_original do |original_method, *args, &block| + # set a value for any calls originating from the migration file, not from rails itself + original_method.call(*args, &block).dup.tap {|i| i[:password] ||= "abc" if caller_locations.any? {|loc| loc.path.include?("db/migrate/20241017013023_reencrypt_password_scramsha.rb")} } end expect(ActiveRecord::Base.connection).to receive(:execute).with(a_string_matching(/ALTER ROLE #{username} WITH PASSWORD \'SCRAM-SHA-256.*\'\;/)) @@ -23,9 +23,9 @@ username = ActiveRecord::Base.connection_db_config.configuration_hash[:username] - expect(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).exactly(10).times.and_call_original - expect(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).and_wrap_original do |original_method, *args, &block| - original_method.call(*args, &block).dup.tap { |i| i.delete(:password) } + allow(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).and_wrap_original do |original_method, *args, &block| + # set a value for any calls originating from the migration file, not from rails itself + original_method.call(*args, &block).dup.tap { |i| i.delete(:password) if caller_locations.any? {|loc| loc.path.include?("db/migrate/20241017013023_reencrypt_password_scramsha.rb")} } end expect(ActiveRecord::Base.connection).not_to receive(:execute).with(a_string_matching(/ALTER ROLE.*\'\;/))