Skip to content

Commit

Permalink
Allow the rails method to be called, don't expect it
Browse files Browse the repository at this point in the history
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 742841c 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
  • Loading branch information
jrafanie committed Dec 12, 2024
1 parent d08bbf3 commit 7af2420
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*\'\;/))

Expand All @@ -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.*\'\;/))

Expand Down

0 comments on commit 7af2420

Please sign in to comment.