Skip to content

Test timing analysis #319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
12 changes: 10 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
crdb: [v23.1.11]
crdb: [v23.1.15]
ruby: [head]
name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }})
steps:
Expand Down Expand Up @@ -95,4 +95,12 @@ jobs:
SET CLUSTER SETTING sql.defaults.experimental_temporary_tables.enabled = 'true';
"
- name: Test
run: bundle exec rake test TESTOPTS='--profile=3'
run: bundle exec rake test TESTOPTS='--profile=3 --stackprof --profile-setup'
- name: Save timing artifacts
if: ${{ always() }}
uses: actions/upload-artifact@v4
with:
name: performance
path: |
sql.ljson
stackprof-minitest-*.dump
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.1
3.2.3
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ source "https://rubygems.org"

gemspec

gem "minitest-stackprof"

module RailsTag
class << self
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,36 @@ def check_all_foreign_keys_valid!
end

def disable_referential_integrity
foreign_keys = tables.map { |table| foreign_keys(table) }.flatten
# foreign_keys = tables.map { |table| foreign_keys(table) }.flatten

foreign_keys.each do |foreign_key|
remove_foreign_key(foreign_key.from_table, name: foreign_key.options[:name])
end
# foreign_keys.each do |foreign_key|
# remove_foreign_key(foreign_key.from_table, name: foreign_key.options[:name])
# end

yield

# Prefixes and suffixes are added in add_foreign_key
# in AR7+ so we need to temporarily disable them here,
# otherwise prefixes/suffixes will be erroneously added.
old_prefix = ActiveRecord::Base.table_name_prefix
old_suffix = ActiveRecord::Base.table_name_suffix

ActiveRecord::Base.table_name_prefix = ""
ActiveRecord::Base.table_name_suffix = ""

begin
foreign_keys.each do |foreign_key|
# Avoid having PG:DuplicateObject error if a test is ran in transaction.
# TODO: verify that there is no cache issue related to running this (e.g: fk
# still in cache but not in db)
next if foreign_key_exists?(foreign_key.from_table, name: foreign_key.options[:name])

add_foreign_key(foreign_key.from_table, foreign_key.to_table, **foreign_key.options)
end
ensure
ActiveRecord::Base.table_name_prefix = old_prefix
ActiveRecord::Base.table_name_suffix = old_suffix
end
# old_prefix = ActiveRecord::Base.table_name_prefix
# old_suffix = ActiveRecord::Base.table_name_suffix

# ActiveRecord::Base.table_name_prefix = ""
# ActiveRecord::Base.table_name_suffix = ""

# begin
# foreign_keys.each do |foreign_key|
# # Avoid having PG:DuplicateObject error if a test is ran in transaction.
# # TODO: verify that there is no cache issue related to running this (e.g: fk
# # still in cache but not in db)
# next if foreign_key_exists?(foreign_key.from_table, name: foreign_key.options[:name])

# add_foreign_key(foreign_key.from_table, foreign_key.to_table, **foreign_key.options)
# end
# ensure
# ActiveRecord::Base.table_name_prefix = old_prefix
# ActiveRecord::Base.table_name_suffix = old_suffix
# end
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions test/cases/helper_cockroachdb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ def load_schema
end
LoadSchemaHelper.prepend(LoadSchemaHelperExt)

require "active_record"
require "active_record/connection_adapters"
require "active_record/connection_adapters/postgresql/schema_statements"
require "active_record/connection_adapters/cockroachdb/schema_statements"

# Disable foreign_keys altogether.
ActiveRecord::ConnectionAdapters::CockroachDB::SchemaStatements.prepend(Module.new do
def use_foreign_keys?; false; end
end)


# Load ActiveRecord test helper
require "cases/helper"

Expand Down Expand Up @@ -176,3 +187,14 @@ def header(stream)
end

ActiveRecord::SchemaDumper.prepend(NoHeaderExt)


ActiveSupport::Notifications.subscribe 'sql.active_record' do |*args|
event = ActiveSupport::Notifications::Event.new(*args)
sql = event.payload[:sql]
duration = event.duration # ms

File.open("sql.ljson", "a") do |f|
f.puts JSON.dump({sql: sql, duration: duration})
end
end
14 changes: 10 additions & 4 deletions test/cases/migration/foreign_key_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,11 @@ def test_add_foreign_key_with_prefix
silence_stream($stdout) { migration.migrate(:up) }
assert_equal 1, @connection.foreign_keys("p_classes").size
ensure
silence_stream($stdout) { migration.migrate(:down) }
ActiveRecord::Base.table_name_prefix = nil
begin
silence_stream($stdout) { migration.migrate(:down) }
ensure
ActiveRecord::Base.table_name_prefix = nil
end
end

def test_add_foreign_key_with_suffix
Expand All @@ -291,8 +294,11 @@ def test_add_foreign_key_with_suffix
silence_stream($stdout) { migration.migrate(:up) }
assert_equal 1, @connection.foreign_keys("classes_s").size
ensure
silence_stream($stdout) { migration.migrate(:down) }
ActiveRecord::Base.table_name_suffix = nil
begin
silence_stream($stdout) { migration.migrate(:down) }
ensure
ActiveRecord::Base.table_name_suffix = nil
end
end

def test_remove_foreign_key_with_if_exists_not_set
Expand Down