From a62542d41095f6ca30a35bf106c91bb9d88b51f1 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Wed, 4 Sep 2024 14:01:54 +0200 Subject: [PATCH 01/33] feat: bump activerecord --- .github/workflows/tests.yml | 4 ++- activerecord-postgis-adapter.gemspec | 5 +-- bin/console | 33 +++++++++++++++++++ .../postgis/create_connection.rb | 27 --------------- .../connection_adapters/postgis_adapter.rb | 1 - lib/activerecord-postgis-adapter.rb | 2 +- 6 files changed, 40 insertions(+), 32 deletions(-) create mode 100755 bin/console delete mode 100644 lib/active_record/connection_adapters/postgis/create_connection.rb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 824c9547..ac470da8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -27,7 +27,9 @@ jobs: strategy: fail-fast: false matrix: - ruby: [ruby-head, '3.3', '3.2', '3.1'] + # ruby-lang.org/en/downloads/branches + ruby: [ruby-head, "3.3", "3.2", "3.1"] + # https://www.postgresql.org/support/versioning/ pg: [12-master, 13-master, 14-master, 15-master, 16-master] steps: - name: Set Up Actions diff --git a/activerecord-postgis-adapter.gemspec b/activerecord-postgis-adapter.gemspec index 5b1fbc1c..9c16f34f 100644 --- a/activerecord-postgis-adapter.gemspec +++ b/activerecord-postgis-adapter.gemspec @@ -10,16 +10,17 @@ Gem::Specification.new do |spec| spec.version = ActiveRecord::ConnectionAdapters::PostGIS::VERSION spec.authors = ["Daniel Azuma", "Tee Parham"] - spec.email = ["dazuma@gmail.com", "parhameter@gmail.com", "kfdoggett@gmail.com"] + spec.email = ["kfdoggett@gmail.com", "buonomo.ulysse@gmail.com"] spec.homepage = "http://github.com/rgeo/activerecord-postgis-adapter" spec.license = "BSD-3-Clause" spec.files = Dir["lib/**/*", "LICENSE.txt"] spec.platform = Gem::Platform::RUBY + # # ruby-lang.org/en/downloads/branches spec.required_ruby_version = ">= 3.1.0" - spec.add_dependency "activerecord", "~> 7.2.0.beta2" + spec.add_dependency "activerecord", "~> 7.2.0" spec.add_dependency "rgeo-activerecord", "~> 7.0.0" spec.add_development_dependency "rake", "~> 13.0" diff --git a/bin/console b/bin/console new file mode 100755 index 00000000..499d0987 --- /dev/null +++ b/bin/console @@ -0,0 +1,33 @@ +#!/usr/bin/env ruby + +$:.unshift(File.expand_path("../lib", __dir__)) + +# require "bundler/setup" +# Bundler.require :development +# +require "activerecord-postgis-adapter" + +db_name = "activerecord_postgis_adapter_console" +system("psql -c 'drop database if exists #{db_name}' postgres", + exception: true) +system("psql -c 'create database #{db_name}' postgres", + exception: true) +system(psql = "psql -c 'create extension postgis' #{db_name} ", exception: true) + +ActiveRecord::Base.establish_connection( + adapter: "postgis", + database: db_name +) + +class Country < ActiveRecord::Base +end + +ActiveRecord::Schema.define do + create_table(:countries) do |t| + t.st_polygon(:area, srid: 4326) + t.string(:name) + end +end + +require "irb" +IRB.start(__FILE__) diff --git a/lib/active_record/connection_adapters/postgis/create_connection.rb b/lib/active_record/connection_adapters/postgis/create_connection.rb deleted file mode 100644 index f56854b6..00000000 --- a/lib/active_record/connection_adapters/postgis/create_connection.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord # :nodoc: - module ConnectionHandling # :nodoc: - # Based on the default postgresql_connection definition from ActiveRecord. - # https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb - # FULL REPLACEMENT because we need to create a different class. - def postgis_connection(config) - conn_params = config.symbolize_keys.compact - - # Map ActiveRecords param names to PGs. - conn_params[:user] = conn_params.delete(:username) if conn_params[:username] - conn_params[:dbname] = conn_params.delete(:database) if conn_params[:database] - - # Forward only valid config params to PG.connect - valid_conn_param_keys = PG::Connection.conndefaults_hash.keys + [:requiressl] - conn_params.slice!(*valid_conn_param_keys) - - ConnectionAdapters::PostGISAdapter.new( - ConnectionAdapters::PostGISAdapter.new_client(conn_params), - logger, - conn_params, - config - ) - end - end -end diff --git a/lib/active_record/connection_adapters/postgis_adapter.rb b/lib/active_record/connection_adapters/postgis_adapter.rb index 8cd45453..9b890d55 100644 --- a/lib/active_record/connection_adapters/postgis_adapter.rb +++ b/lib/active_record/connection_adapters/postgis_adapter.rb @@ -17,7 +17,6 @@ require_relative "postgis/oid/spatial" require_relative "postgis/oid/date_time" require_relative "postgis/type" # has to be after oid/* -require_relative "postgis/create_connection" # :startdoc: module ActiveRecord diff --git a/lib/activerecord-postgis-adapter.rb b/lib/activerecord-postgis-adapter.rb index 7f6387bf..c0dce484 100644 --- a/lib/activerecord-postgis-adapter.rb +++ b/lib/activerecord-postgis-adapter.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'active_record' +require "active_record" require "active_record/connection_adapters" require "rgeo/active_record" ActiveRecord::ConnectionAdapters.register("postgis", "ActiveRecord::ConnectionAdapters::PostGISAdapter", "active_record/connection_adapters/postgis_adapter") From c21dbceb6e4971727c86abf00099a6257a3e3172 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Wed, 4 Sep 2024 14:08:27 +0200 Subject: [PATCH 02/33] feat: enable all tests --- .github/workflows/tests.yml | 31 ++++++++++++++++++++++++------- Rakefile | 2 +- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ac470da8..a3e5653f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -4,9 +4,20 @@ on: branches: - master pull_request: + types: [opened, reopened, synchronize] + # Allows you to run this workflow manually from the Actions tab + workflow_dispatch: jobs: - test-ubuntu-ruby: + # Since the name of the matrix job depends on the version, we define another job with a more stable name. + test_results: + if: ${{ always() }} + runs-on: ubuntu-latest + name: Test Results + needs: [test] + steps: + - run: '[[ "${{ needs.test.result }}" == "success" ]]' + test: runs-on: ubuntu-latest services: postgis: @@ -27,8 +38,8 @@ jobs: strategy: fail-fast: false matrix: - # ruby-lang.org/en/downloads/branches - ruby: [ruby-head, "3.3", "3.2", "3.1"] + # https://ruby-lang.org/en/downloads/branches + ruby: ["3.3", "3.2", "3.1"] # https://www.postgresql.org/support/versioning/ pg: [12-master, 13-master, 14-master, 15-master, 16-master] steps: @@ -41,12 +52,18 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true - - name: Create Database - run: psql -d postgresql://postgres:postgres@localhost:5432/postgres -c "create database postgis_adapter_test" - - name: Create PostGIS Extension - run: psql -d postgresql://postgres:postgres@localhost:5432/postgis_adapter_test -c "create extension postgis" + - name: Setup Database + run: | + psql -d postgresql://postgres:postgres@localhost:5432/postgres \ + -c "create database postgis_adapter_test" \ + -c "create database activerecord_unittest" \ + -c "create database activerecord_unittest2" + for db in postgis_adapter_test activerecord_unittest activerecord_unittest2; do + psql -d postgresql://postgres:postgres@localhost:5432/$db -c "create extension postgis" + done - name: Run Tests run: bundle exec rake test env: PGHOST: localhost + PGUSER: postgres PGPASSWORD: postgres diff --git a/Rakefile b/Rakefile index d488078a..7e9a1f10 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,7 @@ require "rake/testtask" require_relative "test/rake_helper" task default: [:test] -task test: "test:postgis" +task test: "test:all" Rake::TestTask.new(:test_postgis) do |t| t.libs << postgis_test_load_paths From 28cab3a616d85d6251674c7053912b0aed618cbb Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Wed, 4 Sep 2024 15:08:26 +0200 Subject: [PATCH 03/33] fix: use lease_connection in tests --- .github/workflows/tests.yml | 1 + activerecord-postgis-adapter.gemspec | 2 +- test/cases/attributes_test.rb | 6 +- test/cases/basic_test.rb | 22 ++-- test/cases/ddl_test.rb | 84 +++++++------- test/cases/nested_class_test.rb | 4 +- test/cases/spatial_queries_test.rb | 2 +- test/rake_helper.rb | 14 +-- test/schema/postgis_specific_schema.rb | 147 ------------------------- test/test_helper.rb | 29 +++-- 10 files changed, 85 insertions(+), 226 deletions(-) delete mode 100644 test/schema/postgis_specific_schema.rb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a3e5653f..4d1419ad 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -67,3 +67,4 @@ jobs: PGHOST: localhost PGUSER: postgres PGPASSWORD: postgres + TESTOPTS: --verbose --name=/PostgresqlHstoreTest/ diff --git a/activerecord-postgis-adapter.gemspec b/activerecord-postgis-adapter.gemspec index 9c16f34f..4e58a310 100644 --- a/activerecord-postgis-adapter.gemspec +++ b/activerecord-postgis-adapter.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |spec| spec.version = ActiveRecord::ConnectionAdapters::PostGIS::VERSION spec.authors = ["Daniel Azuma", "Tee Parham"] - spec.email = ["kfdoggett@gmail.com", "buonomo.ulysse@gmail.com"] + spec.email = ["kfdoggett@gmail.com", "buonomo.ulysse@gmail.com", "terminale@gmail.com"] spec.homepage = "http://github.com/rgeo/activerecord-postgis-adapter" spec.license = "BSD-3-Clause" diff --git a/test/cases/attributes_test.rb b/test/cases/attributes_test.rb index 31cc7f2c..d1642e95 100644 --- a/test/cases/attributes_test.rb +++ b/test/cases/attributes_test.rb @@ -99,12 +99,12 @@ def test_joined_spatial_attribute private def create_foo - Foo.connection.create_table :foos, force: true do |t| + Foo.lease_connection.create_table :foos, force: true do |t| end end def create_spatial_foo - SpatialFoo.connection.create_table :spatial_foos, force: true do |t| + SpatialFoo.lease_connection.create_table :spatial_foos, force: true do |t| t.references :foo t.st_point :geo_point, geographic: true, srid: 4326 t.st_point :cart_point, srid: 3509 @@ -112,7 +112,7 @@ def create_spatial_foo end def create_invalid_attributes - InvalidAttribute.connection.create_table :invalid_attributes, force: true do |t| + InvalidAttribute.lease_connection.create_table :invalid_attributes, force: true do |t| end end end diff --git a/test/cases/basic_test.rb b/test/cases/basic_test.rb index 352ed6f0..b86b80fc 100644 --- a/test/cases/basic_test.rb +++ b/test/cases/basic_test.rb @@ -13,14 +13,14 @@ def test_version end def test_postgis_available - assert_equal "PostGIS", SpatialModel.connection.adapter_name - assert_equal postgis_version, SpatialModel.connection.postgis_lib_version - valid_version = ["2.", "3."].any? { |major_ver| SpatialModel.connection.postgis_lib_version.start_with? major_ver } + assert_equal "PostGIS", SpatialModel.lease_connection.adapter_name + assert_equal SpatialModel.lease_connection.select_value("SELECT postgis_lib_version()"), SpatialModel.lease_connection.postgis_lib_version + valid_version = ["2.", "3."].any? { |major_ver| SpatialModel.lease_connection.postgis_lib_version.start_with? major_ver } assert valid_version end def test_arel_visitor - visitor = Arel::Visitors::PostGIS.new(SpatialModel.connection) + visitor = Arel::Visitors::PostGIS.new(SpatialModel.lease_connection) node = RGeo::ActiveRecord::SpatialConstantNode.new("POINT (1.0 2.0)") collector = Arel::Collectors::PlainString.new visitor.accept(node, collector) @@ -28,7 +28,7 @@ def test_arel_visitor end def test_arel_visitor_will_not_visit_string - visitor = Arel::Visitors::PostGIS.new(SpatialModel.connection) + visitor = Arel::Visitors::PostGIS.new(SpatialModel.lease_connection) node = "POINT (1 2)" collector = Arel::Collectors::PlainString.new @@ -111,7 +111,7 @@ def test_default_value def test_custom_factory klass = SpatialModel - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.st_polygon(:area, srid: 4326) end klass.reset_column_information @@ -127,7 +127,7 @@ def test_custom_factory def test_spatial_factory_attrs_parsing klass = SpatialModel - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.multi_polygon(:areas, srid: 4326) end klass.reset_column_information @@ -152,14 +152,14 @@ def test_readme_example spatial_factory_store.register(geo_factory, geo_type: "point", sql_type: "geography") klass = SpatialModel - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column(:shape, :geometry) t.line_string(:path, srid: 3785) t.st_point(:latlon, geographic: true) end klass.reset_column_information assert_includes klass.columns.map(&:name), "shape" - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.index(:latlon, using: :gist) end @@ -192,7 +192,7 @@ def test_custom_column end def test_multi_polygon_column - SpatialModel.connection.create_table(:spatial_models, force: true) do |t| + SpatialModel.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "m_poly", :multi_polygon end SpatialModel.reset_column_information @@ -212,7 +212,7 @@ def test_multi_polygon_column private def create_model - SpatialModel.connection.create_table(:spatial_models, force: true) do |t| + SpatialModel.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :st_point, srid: 3785 t.column "latlon_geo", :st_point, srid: 4326, geographic: true t.column "default_latlon", :st_point, srid: 0, default: "POINT(0.0 0.0)" diff --git a/test/cases/ddl_test.rb b/test/cases/ddl_test.rb index dc501af5..94890555 100644 --- a/test/cases/ddl_test.rb +++ b/test/cases/ddl_test.rb @@ -21,12 +21,12 @@ def test_spatial_column_options end def test_type_to_sql - adapter = SpatialModel.connection + adapter = SpatialModel.lease_connection assert_equal "geometry(point,4326)", adapter.type_to_sql(:geometry, limit: "point,4326") end def test_create_simple_geometry - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :geometry end klass.reset_column_information @@ -36,12 +36,12 @@ def test_create_simple_geometry assert_equal true, col.spatial? assert_equal false, col.geographic? assert_equal 0, col.srid - klass.connection.drop_table(:spatial_models) + klass.lease_connection.drop_table(:spatial_models) assert_equal 0, count_geometry_columns end def test_create_simple_geography - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :geometry, geographic: true end klass.reset_column_information @@ -54,7 +54,7 @@ def test_create_simple_geography end def test_create_point_geometry - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :st_point end klass.reset_column_information @@ -62,21 +62,21 @@ def test_create_point_geometry end def test_create_geometry_with_index - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :geometry end - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.index([:latlon], using: :gist) end klass.reset_column_information - assert_equal :gist, klass.connection.indexes(:spatial_models).last.using + assert_equal :gist, klass.lease_connection.indexes(:spatial_models).last.using end def test_add_geometry_column - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column("latlon", :geometry) end - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.column("geom2", :st_point, srid: 4326) t.column("name", :string) end @@ -95,7 +95,7 @@ def test_add_geometry_column end def test_add_geometry_column_null_false - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column("latlon_null", :geometry, null: false) t.column("latlon", :geometry) end @@ -108,10 +108,10 @@ def test_add_geometry_column_null_false end def test_add_geography_column - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column("latlon", :geometry) end - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.st_point("geom3", srid: 4326, geographic: true) t.column("geom2", :st_point, srid: 4326, geographic: true) t.column("name", :string) @@ -139,11 +139,11 @@ def test_add_geography_column end def test_drop_geometry_column - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column("latlon", :geometry) t.column("geom2", :st_point, srid: 4326) end - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.remove("geom2") end klass.reset_column_information @@ -156,12 +156,12 @@ def test_drop_geometry_column end def test_drop_geography_column - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column("latlon", :geometry) t.column("geom2", :st_point, srid: 4326, geographic: true) t.column("geom3", :st_point, srid: 4326) end - klass.connection.change_table(:spatial_models) do |t| + klass.lease_connection.change_table(:spatial_models) do |t| t.remove("geom2") end klass.reset_column_information @@ -176,7 +176,7 @@ def test_drop_geography_column end def test_create_simple_geometry_using_shortcut - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.geometry "latlon" end klass.reset_column_information @@ -185,12 +185,12 @@ def test_create_simple_geometry_using_shortcut assert_equal RGeo::Feature::Geometry, col.geometric_type assert_equal false, col.geographic? assert_equal 0, col.srid - klass.connection.drop_table(:spatial_models) + klass.lease_connection.drop_table(:spatial_models) assert_equal 0, count_geometry_columns end def test_create_simple_geography_using_shortcut - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.geometry "latlon", geographic: true end klass.reset_column_information @@ -202,7 +202,7 @@ def test_create_simple_geography_using_shortcut end def test_create_point_geometry_using_shortcut - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.st_point "latlon" end klass.reset_column_information @@ -210,7 +210,7 @@ def test_create_point_geometry_using_shortcut end def test_create_geometry_using_shortcut_with_srid - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.geometry "latlon", srid: 4326 end klass.reset_column_information @@ -220,7 +220,7 @@ def test_create_geometry_using_shortcut_with_srid end def test_create_polygon_with_options - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "region", :st_polygon, has_m: true, srid: 3785 end klass.reset_column_information @@ -232,12 +232,12 @@ def test_create_polygon_with_options assert_equal true, col.has_m? assert_equal 3785, col.srid assert_equal({ has_m: true, type: "st_polygon", srid: 3785 }, col.limit) - klass.connection.drop_table(:spatial_models) + klass.lease_connection.drop_table(:spatial_models) assert_equal 0, count_geometry_columns end def test_create_spatial_column_default_value_geometric - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "coordinates", :st_point, srid: 3875, default: "POINT(0.0 0.0)" end klass.reset_column_information @@ -251,12 +251,12 @@ def test_create_spatial_column_default_value_geometric assert_equal 3875, col.srid assert_equal "010100000000000000000000000000000000000000", col.default assert_equal({ type: "st_point", srid: 3875 }, col.limit) - klass.connection.drop_table(:spatial_models) + klass.lease_connection.drop_table(:spatial_models) assert_equal 0, count_geometry_columns end def test_create_spatial_column_default_value_geographic - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.geography "coordinates", limit: { srid: 4326, type: "st_point", geographic: true }, default: "POINT(0.0 0.0)" end klass.reset_column_information @@ -270,12 +270,12 @@ def test_create_spatial_column_default_value_geographic assert_equal 4326, col.srid assert_equal "0101000020E610000000000000000000000000000000000000", col.default assert_equal({ type: "st_point", srid: 4326, geographic: true }, col.limit) - klass.connection.drop_table(:spatial_models) + klass.lease_connection.drop_table(:spatial_models) assert_equal 0, count_geography_columns end def test_no_query_spatial_column_info - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.string "name" end klass.reset_column_information @@ -289,7 +289,7 @@ def test_no_query_spatial_column_info # Ensure that null contraints info is getting captured like the # normal adapter. def test_null_constraints - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "nulls_allowed", :string, null: true t.column "nulls_disallowed", :string, null: false end @@ -300,7 +300,7 @@ def test_null_constraints # Ensure column default value works like the Postgres adapter. def test_column_defaults - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "sample_integer", :integer, default: -1 end klass.reset_column_information @@ -309,8 +309,8 @@ def test_column_defaults # Ensure virtual column default function works like the Postgres adapter. def test_virtual_column_default_function - skip "Virtual Columns are not supported in this version of PostGIS" unless SpatialModel.connection.supports_virtual_columns? - klass.connection.create_table(:spatial_models, force: true) do |t| + skip "Virtual Columns are not supported in this version of PostGIS" unless SpatialModel.lease_connection.supports_virtual_columns? + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.integer :column1 t.virtual :column2, type: :integer, as: "(column1 + 1)", stored: true end @@ -322,7 +322,7 @@ def test_virtual_column_default_function end def test_column_types - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "sample_integer", :integer t.column "sample_string", :string t.column "latlon", :st_point @@ -334,7 +334,7 @@ def test_column_types end def test_array_columns - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "sample_array", :string, array: true t.column "sample_non_array", :string end @@ -344,7 +344,7 @@ def test_array_columns end def test_reload_dumped_schema - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.geography "latlon1", limit: { srid: 4326, type: "point", geographic: true } end klass.reset_column_information @@ -353,7 +353,7 @@ def test_reload_dumped_schema end def test_non_spatial_column_limits - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.string :foo, limit: 123 end klass.reset_column_information @@ -362,7 +362,7 @@ def test_non_spatial_column_limits end def test_column_comments - klass.connection.create_table(:spatial_models, force: true) do |t| + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.string :sample_comment, comment: "Comment test" end klass.reset_column_information @@ -371,8 +371,8 @@ def test_column_comments end def test_generated_geometry_column - skip "Virtual Columns are not supported in this version of PostGIS" unless SpatialModel.connection.supports_virtual_columns? - klass.connection.create_table(:spatial_models, force: true) do |t| + skip "Virtual Columns are not supported in this version of PostGIS" unless SpatialModel.lease_connection.supports_virtual_columns? + klass.lease_connection.create_table(:spatial_models, force: true) do |t| t.st_point :coordinates, limit: { srid: 4326 } t.virtual :generated_buffer, type: :st_polygon, limit: { srid: 4326 }, as: "ST_Buffer(coordinates, 10)", stored: true end @@ -390,11 +390,11 @@ def klass end def count_geometry_columns - klass.connection.select_value(geo_column_sql("geometry_columns", klass.table_name)).to_i + klass.lease_connection.select_value(geo_column_sql("geometry_columns", klass.table_name)).to_i end def count_geography_columns - klass.connection.select_value(geo_column_sql("geography_columns", klass.table_name)).to_i + klass.lease_connection.select_value(geo_column_sql("geography_columns", klass.table_name)).to_i end def geo_column_sql(postgis_view, table_name) diff --git a/test/cases/nested_class_test.rb b/test/cases/nested_class_test.rb index a1c57e8e..ce54200a 100644 --- a/test/cases/nested_class_test.rb +++ b/test/cases/nested_class_test.rb @@ -14,11 +14,11 @@ class Bar < ActiveRecord::Base end def test_nested_model - Foo::Bar.connection.create_table(:foo_bars, force: true) do |t| + Foo::Bar.lease_connection.create_table(:foo_bars, force: true) do |t| t.column "latlon", :st_point, srid: 3785 end assert_empty Foo::Bar.all - Foo::Bar.connection.drop_table(:foo_bars) + Foo::Bar.lease_connection.drop_table(:foo_bars) end end end diff --git a/test/cases/spatial_queries_test.rb b/test/cases/spatial_queries_test.rb index 137e6928..c0e6a77a 100644 --- a/test/cases/spatial_queries_test.rb +++ b/test/cases/spatial_queries_test.rb @@ -118,7 +118,7 @@ def test_ewkt_parser_query private def create_model - SpatialModel.connection.create_table(:spatial_models, force: true) do |t| + SpatialModel.lease_connection.create_table(:spatial_models, force: true) do |t| t.column "latlon", :st_point, srid: 3785 t.column "points", :multi_point, srid: 3785 t.column "path", :line_string, srid: 3785 diff --git a/test/rake_helper.rb b/test/rake_helper.rb index b2204c74..8819c6d4 100644 --- a/test/rake_helper.rb +++ b/test/rake_helper.rb @@ -29,21 +29,19 @@ def activerecord_test_files .map { |file| File.join ar_root, file.strip } .sort .prepend(POSTGIS_TEST_HELPER) + .then{ FileList[*_1] } else - Dir - .glob("#{ar_root}/test/cases/**/*_test.rb") - .grep_v(%r{/adapters/mysql2/}) - .grep_v(%r{/adapters/sqlite3/}) - .sort - .prepend(POSTGIS_TEST_HELPER) + FileList["#{ar_root}/test/cases/**/*_test.rb"] + .reject { _1.include?("/adapters/") || _1.include?("/encryption/performance") } + .then { |list| FileList[POSTGIS_TEST_HELPER] + list + FileList["#{ar_root}/test/cases/adapters/postgresql/**/*_test.rb"] } end end def postgis_test_files if ENV["POSTGIS_TEST_FILES"] - ENV["POSTGIS_TEST_FILES"].split(",").map(&:strip) + ENV["POSTGIS_TEST_FILES"].split(",").map(&:strip).then { FileList[*_1] } else - Dir.glob("test/cases/**/*_test.rb") + FileList["test/cases/**/*_test.rb"] end end diff --git a/test/schema/postgis_specific_schema.rb b/test/schema/postgis_specific_schema.rb deleted file mode 100644 index 535b2153..00000000 --- a/test/schema/postgis_specific_schema.rb +++ /dev/null @@ -1,147 +0,0 @@ -# frozen_string_literal: true - -# This is a copy of postgresql_specific_schema.rb from ActiveRecord's test -# suite. - -ActiveRecord::Schema.define do - ActiveRecord::TestCase.enable_extension!("uuid-ossp", ActiveRecord::Base.connection) - ActiveRecord::TestCase.enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? - - uuid_default = connection.supports_pgcrypto_uuid? ? {} : { default: "uuid_generate_v4()" } - - create_table :uuid_parents, id: :uuid, force: true, **uuid_default do |t| - t.string :name - end - - create_table :uuid_children, id: :uuid, force: true, **uuid_default do |t| - t.string :name - t.uuid :uuid_parent_id - end - - create_table :defaults, force: true do |t| - t.date :modified_date, default: -> { "CURRENT_DATE" } - t.date :modified_date_function, default: -> { "now()" } - t.date :fixed_date, default: "2004-01-01" - t.datetime :modified_time, default: -> { "CURRENT_TIMESTAMP" } - t.datetime :modified_time_without_precision, precision: nil, default: -> { "CURRENT_TIMESTAMP" } - t.datetime :modified_time_with_precision_0, precision: 0, default: -> { "CURRENT_TIMESTAMP" } - t.datetime :modified_time_function, default: -> { "now()" } - t.datetime :fixed_time, default: "2004-01-01 00:00:00.000000-00" - t.timestamptz :fixed_time_with_time_zone, default: "2004-01-01 01:00:00+1" - t.column :char1, "char(1)", default: "Y" - t.string :char2, limit: 50, default: "a varchar field" - t.text :char3, default: "a text field" - t.bigint :bigint_default, default: -> { "0::bigint" } - t.text :multiline_default, default: "--- [] - -" - end - - create_table :postgresql_times, force: true do |t| - t.interval :time_interval - t.interval :scaled_time_interval, precision: 6 - end - - create_table :postgresql_oids, force: true do |t| - t.oid :obj_id - end - - drop_table "postgresql_timestamp_with_zones", if_exists: true - drop_table "postgresql_partitioned_table", if_exists: true - drop_table "postgresql_partitioned_table_parent", if_exists: true - - execute "DROP SEQUENCE IF EXISTS companies_nonstd_seq CASCADE" - execute "CREATE SEQUENCE companies_nonstd_seq START 101 OWNED BY companies.id" - execute "ALTER TABLE companies ALTER COLUMN id SET DEFAULT nextval('companies_nonstd_seq')" - execute "DROP SEQUENCE IF EXISTS companies_id_seq" - - execute "DROP FUNCTION IF EXISTS partitioned_insert_trigger()" - - %w(accounts_id_seq developers_id_seq projects_id_seq topics_id_seq customers_id_seq orders_id_seq).each do |seq_name| - execute "SELECT setval('#{seq_name}', 100)" - end - - execute <<_SQL - CREATE TABLE postgresql_timestamp_with_zones ( - id SERIAL PRIMARY KEY, - time TIMESTAMP WITH TIME ZONE - ); -_SQL - - begin - execute <<_SQL - CREATE TABLE postgresql_partitioned_table_parent ( - id SERIAL PRIMARY KEY, - number integer - ); - CREATE TABLE postgresql_partitioned_table ( ) - INHERITS (postgresql_partitioned_table_parent); - - CREATE OR REPLACE FUNCTION partitioned_insert_trigger() - RETURNS TRIGGER AS $$ - BEGIN - INSERT INTO postgresql_partitioned_table VALUES (NEW.*); - RETURN NULL; - END; - $$ - LANGUAGE plpgsql; - - CREATE TRIGGER insert_partitioning_trigger - BEFORE INSERT ON postgresql_partitioned_table_parent - FOR EACH ROW EXECUTE PROCEDURE partitioned_insert_trigger(); -_SQL - rescue ActiveRecord::StatementInvalid => e - if e.message.include?('language "plpgsql" does not exist') - execute "CREATE LANGUAGE 'plpgsql';" - retry - else - raise e - end - end - - # This table is to verify if the :limit option is being ignored for text and binary columns - create_table :limitless_fields, force: true do |t| - t.binary :binary, limit: 100_000 - t.text :text, limit: 100_000 - end - - create_table :bigint_array, force: true do |t| - t.integer :big_int_data_points, limit: 8, array: true - t.decimal :decimal_array_default, array: true, default: [1.23, 3.45] - end - - create_table :uuid_comments, force: true, id: false do |t| - t.uuid :uuid, primary_key: true, **uuid_default - t.string :content - end - - create_table :uuid_entries, force: true, id: false do |t| - t.uuid :uuid, primary_key: true, **uuid_default - t.string :entryable_type, null: false - t.uuid :entryable_uuid, null: false - end - - create_table :uuid_items, force: true, id: false do |t| - t.uuid :uuid, primary_key: true, **uuid_default - t.string :title - end - - create_table :uuid_messages, force: true, id: false do |t| - t.uuid :uuid, primary_key: true, **uuid_default - t.string :subject - end - - if supports_partitioned_indexes? - create_table(:measurements, id: false, force: true, options: "PARTITION BY LIST (city_id)") do |t| - t.string :city_id, null: false - t.date :logdate, null: false - t.integer :peaktemp - t.integer :unitsales - t.index [:logdate, :city_id], unique: true - end - create_table(:measurements_toronto, id: false, force: true, - options: "PARTITION OF measurements FOR VALUES IN (1)") - create_table(:measurements_concepcion, id: false, force: true, - options: "PARTITION OF measurements FOR VALUES IN (2)") - end -end diff --git a/test/test_helper.rb b/test/test_helper.rb index 18bfdfec..ce5067ee 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -17,7 +17,7 @@ def load_postgis_specific_schema original_stdout = $stdout $stdout = StringIO.new - load "schema/postgis_specific_schema.rb" + load SCHEMA_ROOT + "/postgresql_specific_schema.rb" ActiveRecord::FixtureSet.reset_cache ensure @@ -44,7 +44,7 @@ def with_postgresql_datetime_type(type) else module ActiveRecord class Base - DATABASE_CONFIG_PATH = File.dirname(__FILE__) << "/database.yml" + DATABASE_CONFIG_PATH = __dir__ + "/database.yml" def self.test_connection_hash conns = YAML.load(ERB.new(File.read(DATABASE_CONFIG_PATH)).result) @@ -59,22 +59,29 @@ def self.establish_test_connection end ActiveRecord::Base.establish_test_connection -end +end # end if ENV["ARCONN"] class SpatialModel < ActiveRecord::Base end -module ActiveSupport - class TestCase - self.test_order = :sorted +require 'timeout' - def database_version - @database_version ||= SpatialModel.connection.select_value("SELECT version()") - end +module TestTimeoutHelper + def time_it + t0 = Minitest.clock_time - def postgis_version - @postgis_version ||= SpatialModel.connection.select_value("SELECT postgis_lib_version()") + timeout = ENV.fetch("TEST_TIMEOUT", 10).to_i + Timeout.timeout(timeout, Timeout::Error, "Test took over #{timeout} seconds to finish") do + yield end + ensure + self.time = Minitest.clock_time - t0 + end +end + +module ActiveSupport + class TestCase + include TestTimeoutHelper def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) From b4ed5bf42ad465a97a8a565deee62ccca3a0462d Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 09:33:13 +0200 Subject: [PATCH 04/33] add tracer to figure it out --- Gemfile | 2 ++ test/test_helper.rb | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 5976f205..e125092c 100644 --- a/Gemfile +++ b/Gemfile @@ -32,6 +32,8 @@ end gem "rails", github: "rails/rails", tag: "v#{activerecord_version}" group :development do + gem "tracer" + # Gems used by the ActiveRecord test suite gem "bcrypt" gem "sqlite3" diff --git a/test/test_helper.rb b/test/test_helper.rb index ce5067ee..290180d3 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -65,6 +65,7 @@ class SpatialModel < ActiveRecord::Base end require 'timeout' +require 'tracer' module TestTimeoutHelper def time_it @@ -72,7 +73,9 @@ def time_it timeout = ENV.fetch("TEST_TIMEOUT", 10).to_i Timeout.timeout(timeout, Timeout::Error, "Test took over #{timeout} seconds to finish") do - yield + Tracer.trace_call do + yield + end end ensure self.time = Minitest.clock_time - t0 From bc31a283efb7bc32632787e0bd03c392cefa7e15 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 10:38:51 +0200 Subject: [PATCH 05/33] stackprof less verbose --- Gemfile | 2 +- test/test_helper.rb | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index e125092c..0fa56b84 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ end gem "rails", github: "rails/rails", tag: "v#{activerecord_version}" group :development do - gem "tracer" + gem "stackprof" # Gems used by the ActiveRecord test suite gem "bcrypt" diff --git a/test/test_helper.rb b/test/test_helper.rb index 290180d3..0f326de6 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -65,7 +65,7 @@ class SpatialModel < ActiveRecord::Base end require 'timeout' -require 'tracer' +require 'stackprof' module TestTimeoutHelper def time_it @@ -73,9 +73,11 @@ def time_it timeout = ENV.fetch("TEST_TIMEOUT", 10).to_i Timeout.timeout(timeout, Timeout::Error, "Test took over #{timeout} seconds to finish") do - Tracer.trace_call do + profile = StackProf.run(mode: :wall, interval: 1000) do yield - end + end + puts + StackProf::Report.new(profile).print_text end ensure self.time = Minitest.clock_time - t0 From 5fd9f6c60cb692bdc7bbc64b1bdbe5cb57246ba3 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 11:12:02 +0200 Subject: [PATCH 06/33] enable_ext analysis --- .github/workflows/tests.yml | 5 +++++ test/test_helper.rb | 20 +++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4d1419ad..7acc60e2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,6 +8,11 @@ on: # Allows you to run this workflow manually from the Actions tab workflow_dispatch: +# This allows a subsequently queued workflow run to interrupt previous runs. +concurrency: + group: "${{ github.workflow }} @ ${{ github.ref }}" + cancel-in-progress: true + jobs: # Since the name of the matrix job depends on the version, we define another job with a more stable name. test_results: diff --git a/test/test_helper.rb b/test/test_helper.rb index 0f326de6..51c8102b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -84,9 +84,27 @@ def time_it end end -module ActiveSupport + +module DebugSlowTests + def enable_extension!(...) + t0 = Minitest.clock_time + super + puts "enable_extension! took #{Minitest.clock_time - t0} seconds" + end + + def disable_extension!(...) + t0 = Minitest.clock_time + super + puts "disable_extension! took #{Minitest.clock_time - t0} seconds" + end +end + + +module ActiveRecord class TestCase include TestTimeoutHelper + include DebugSlowTests + extend DebugSlowTests def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) From 3be96f94361c5af185a7b76df2da412e8c104c74 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 11:37:12 +0200 Subject: [PATCH 07/33] enable_ext stackprof --- test/test_helper.rb | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 51c8102b..9750255f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -73,11 +73,7 @@ def time_it timeout = ENV.fetch("TEST_TIMEOUT", 10).to_i Timeout.timeout(timeout, Timeout::Error, "Test took over #{timeout} seconds to finish") do - profile = StackProf.run(mode: :wall, interval: 1000) do - yield - end - puts - StackProf::Report.new(profile).print_text + yield end ensure self.time = Minitest.clock_time - t0 @@ -86,20 +82,33 @@ def time_it module DebugSlowTests - def enable_extension!(...) + def wrap_the_thing(name) + rv = nil t0 = Minitest.clock_time - super - puts "enable_extension! took #{Minitest.clock_time - t0} seconds" + profile = StackProf.run(mode: :wall, interval: 1000) do + rv = yield + end + puts + puts "#{name} took #{Minitest.clock_time - t0} seconds" + puts + StackProf::Report.new(profile).print_text + rv + end + def enable_extension!(...) + wrap_the_thing(__method__) do + super + end end def disable_extension!(...) - t0 = Minitest.clock_time - super - puts "disable_extension! took #{Minitest.clock_time - t0} seconds" + wrap_the_thing(__method__) do + super + end end end + module ActiveRecord class TestCase include TestTimeoutHelper From eee50f618e6cfb78f0221ece6aacf93c8d8e2cd2 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 12:19:25 +0200 Subject: [PATCH 08/33] show conninfo --- test/database.yml | 2 +- test/test_helper.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/database.yml b/test/database.yml index 47966b34..b7980531 100644 --- a/test/database.yml +++ b/test/database.yml @@ -8,7 +8,7 @@ connections: password: <%= ENV["PGPASSWORD"] || "" %> setup: default schema_search_path: public - aruint2: + arunit2: host: <%= ENV["PGHOST"] || "127.0.0.1" %> port: <%= ENV["PGPORT"] || "5432" %> database: <%= ENV["PGDATABASE"] || "postgis_adapter_test" %> diff --git a/test/test_helper.rb b/test/test_helper.rb index 9750255f..24ae171c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -91,6 +91,8 @@ def wrap_the_thing(name) puts puts "#{name} took #{Minitest.clock_time - t0} seconds" puts + pp SpatialModel.lease_connection.instance_variable_get(:@raw_connection).conninfo_hash + puts StackProf::Report.new(profile).print_text rv end From ec1c5efddd64e14fc70ca1af2cc7ae235887161d Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 12:47:42 +0200 Subject: [PATCH 09/33] debug conninfo --- .github/workflows/tests.yml | 4 ++-- test/test_helper.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7acc60e2..d6cb8287 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,9 +44,9 @@ jobs: fail-fast: false matrix: # https://ruby-lang.org/en/downloads/branches - ruby: ["3.3", "3.2", "3.1"] + ruby: ["3.3"] # https://www.postgresql.org/support/versioning/ - pg: [12-master, 13-master, 14-master, 15-master, 16-master] + pg: [16-master] steps: - name: Set Up Actions uses: actions/checkout@v4 diff --git a/test/test_helper.rb b/test/test_helper.rb index 24ae171c..cd650116 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -135,3 +135,15 @@ def reset_spatial_store end end end + +conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) +$count = conn.conninfo_hash[:port].count(",")+1 + +TracePoint.trace(:call) do |tp| + conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) + count = conn.conninfo_hash[:port].count(",")+1 + next if count == $count + + $count = count + puts "port(count=#{count}): #{conn.conninfo_hash[:port][0, 100]}" +end From 342fcb27bde9b81057902d0761a322d9e9a0f8df Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 12:58:19 +0200 Subject: [PATCH 10/33] maybe reset is the culprit --- test/test_helper.rb | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index cd650116..4c06b872 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -136,14 +136,32 @@ def reset_spatial_store end end -conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) -$count = conn.conninfo_hash[:port].count(",")+1 +# conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) +# $count = conn.conninfo_hash[:port].count(",")+1 -TracePoint.trace(:call) do |tp| - conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) - count = conn.conninfo_hash[:port].count(",")+1 - next if count == $count +# TracePoint.trace(:call) do |tp| +# conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) +# count = conn.conninfo_hash[:port].count(",")+1 +# next if count == $count - $count = count - puts "port(count=#{count}): #{conn.conninfo_hash[:port][0, 100]}" +# $count = count +# puts "port(count=#{count}): #{conn.conninfo_hash[:port][0, 100]}" +# end + +module DebugReset + def reset + + iopts = conninfo_hash.compact + puts "host count before: #{iopts[:host].count(",") + 1}" + if iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000 + iopts = self.class.send(:resolve_hosts, iopts) + end + puts "host count after: #{iopts[:host].count(",") + 1}" + conninfo = self.class.parse_connect_args( iopts ); + reset_start2(conninfo) + async_connect_or_reset(:reset_poll) + self + end end + +PG::Connection.prepend(DebugReset) From 4951a6b1feabfcb7934745e90ea914399d2367d5 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 13:06:40 +0200 Subject: [PATCH 11/33] closer --- test/test_helper.rb | 73 +++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 4c06b872..0bebbafd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -81,41 +81,41 @@ def time_it end -module DebugSlowTests - def wrap_the_thing(name) - rv = nil - t0 = Minitest.clock_time - profile = StackProf.run(mode: :wall, interval: 1000) do - rv = yield - end - puts - puts "#{name} took #{Minitest.clock_time - t0} seconds" - puts - pp SpatialModel.lease_connection.instance_variable_get(:@raw_connection).conninfo_hash - puts - StackProf::Report.new(profile).print_text - rv - end - def enable_extension!(...) - wrap_the_thing(__method__) do - super - end - end - - def disable_extension!(...) - wrap_the_thing(__method__) do - super - end - end -end +# module DebugSlowTests +# def wrap_the_thing(name) +# rv = nil +# t0 = Minitest.clock_time +# profile = StackProf.run(mode: :wall, interval: 1000) do +# rv = yield +# end +# puts +# puts "#{name} took #{Minitest.clock_time - t0} seconds" +# puts +# pp SpatialModel.lease_connection.instance_variable_get(:@raw_connection).conninfo_hash +# puts +# StackProf::Report.new(profile).print_text +# rv +# end +# def enable_extension!(...) +# wrap_the_thing(__method__) do +# super +# end +# end + +# def disable_extension!(...) +# wrap_the_thing(__method__) do +# super +# end +# end +# end module ActiveRecord class TestCase include TestTimeoutHelper - include DebugSlowTests - extend DebugSlowTests + # include DebugSlowTests + # extend DebugSlowTests def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) @@ -164,4 +164,19 @@ def reset end end +module DebugResolve +def resolve_hosts(iopts) + host = iopts[:host] + host = host[0, 97] + "..." if host.length > 100 + puts "resolve_hosts, hosts: #{host.inspect}" + + port = iopts[:port] + port = port[0, 97] + "..." if port.length > 100 + puts "resolve_hosts, ports: #{port.inspect}" + + super + end +end + PG::Connection.prepend(DebugReset) +PG::Connection.singleton_class.prepend(DebugResolve) From b774ae24fe9dfb974fbe5ffde52bfb53fdec21a5 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 13:32:53 +0200 Subject: [PATCH 12/33] fixed maybe --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d6cb8287..9e19fc6f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -69,7 +69,7 @@ jobs: - name: Run Tests run: bundle exec rake test env: - PGHOST: localhost + PGHOST: 127.0.0.1 PGUSER: postgres PGPASSWORD: postgres TESTOPTS: --verbose --name=/PostgresqlHstoreTest/ From d1a4e1e8f750b72e0500e83d4c28e9734f68af2a Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 13:40:30 +0200 Subject: [PATCH 13/33] cleanup --- .github/workflows/tests.yml | 6 +-- test/test_helper.rb | 78 ------------------------------------- 2 files changed, 3 insertions(+), 81 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9e19fc6f..6fe5b0bc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,9 +44,9 @@ jobs: fail-fast: false matrix: # https://ruby-lang.org/en/downloads/branches - ruby: ["3.3"] + ruby: ["3.3", "3.2", "3.1"] # https://www.postgresql.org/support/versioning/ - pg: [16-master] + pg: [12-master, 13-master, 14-master, 15-master, 16-master] steps: - name: Set Up Actions uses: actions/checkout@v4 @@ -72,4 +72,4 @@ jobs: PGHOST: 127.0.0.1 PGUSER: postgres PGPASSWORD: postgres - TESTOPTS: --verbose --name=/PostgresqlHstoreTest/ + TESTOPTS: --verbose --profile=3 diff --git a/test/test_helper.rb b/test/test_helper.rb index 0bebbafd..9621873b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -80,42 +80,9 @@ def time_it end end - -# module DebugSlowTests -# def wrap_the_thing(name) -# rv = nil -# t0 = Minitest.clock_time -# profile = StackProf.run(mode: :wall, interval: 1000) do -# rv = yield -# end -# puts -# puts "#{name} took #{Minitest.clock_time - t0} seconds" -# puts -# pp SpatialModel.lease_connection.instance_variable_get(:@raw_connection).conninfo_hash -# puts -# StackProf::Report.new(profile).print_text -# rv -# end -# def enable_extension!(...) -# wrap_the_thing(__method__) do -# super -# end -# end - -# def disable_extension!(...) -# wrap_the_thing(__method__) do -# super -# end -# end -# end - - - module ActiveRecord class TestCase include TestTimeoutHelper - # include DebugSlowTests - # extend DebugSlowTests def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) @@ -135,48 +102,3 @@ def reset_spatial_store end end end - -# conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) -# $count = conn.conninfo_hash[:port].count(",")+1 - -# TracePoint.trace(:call) do |tp| -# conn = SpatialModel.lease_connection.instance_variable_get(:@raw_connection) -# count = conn.conninfo_hash[:port].count(",")+1 -# next if count == $count - -# $count = count -# puts "port(count=#{count}): #{conn.conninfo_hash[:port][0, 100]}" -# end - -module DebugReset - def reset - - iopts = conninfo_hash.compact - puts "host count before: #{iopts[:host].count(",") + 1}" - if iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000 - iopts = self.class.send(:resolve_hosts, iopts) - end - puts "host count after: #{iopts[:host].count(",") + 1}" - conninfo = self.class.parse_connect_args( iopts ); - reset_start2(conninfo) - async_connect_or_reset(:reset_poll) - self - end -end - -module DebugResolve -def resolve_hosts(iopts) - host = iopts[:host] - host = host[0, 97] + "..." if host.length > 100 - puts "resolve_hosts, hosts: #{host.inspect}" - - port = iopts[:port] - port = port[0, 97] + "..." if port.length > 100 - puts "resolve_hosts, ports: #{port.inspect}" - - super - end -end - -PG::Connection.prepend(DebugReset) -PG::Connection.singleton_class.prepend(DebugResolve) From 2eef5aa2463e2172c6af7ce9cda8a55991ea8b17 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 15:18:10 +0200 Subject: [PATCH 14/33] add exclusions --- Gemfile | 2 +- activerecord-postgis-adapter.gemspec | 1 - test/excludes/MultiDbMigratorTest.rb | 4 ++++ test/test_helper.rb | 4 ++-- 4 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 test/excludes/MultiDbMigratorTest.rb diff --git a/Gemfile b/Gemfile index 0fa56b84..7fe2ecb8 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ end gem "rails", github: "rails/rails", tag: "v#{activerecord_version}" group :development do - gem "stackprof" + gem "minitest-excludes", "~> 2.0" # Gems used by the ActiveRecord test suite gem "bcrypt" diff --git a/activerecord-postgis-adapter.gemspec b/activerecord-postgis-adapter.gemspec index 4e58a310..0378c05c 100644 --- a/activerecord-postgis-adapter.gemspec +++ b/activerecord-postgis-adapter.gemspec @@ -25,7 +25,6 @@ Gem::Specification.new do |spec| spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "minitest", "~> 5.4" - spec.add_development_dependency "mocha", "~> 2.4" spec.add_development_dependency "benchmark-ips", "~> 2.12" spec.add_development_dependency "rubocop", "~> 1.50" diff --git a/test/excludes/MultiDbMigratorTest.rb b/test/excludes/MultiDbMigratorTest.rb new file mode 100644 index 00000000..2a06a9a7 --- /dev/null +++ b/test/excludes/MultiDbMigratorTest.rb @@ -0,0 +1,4 @@ +exclude :test_migrator_forward, "Hangs indefinitely. Doesn't care about timeout" +exclude :test_get_all_versions, "Hangs indefinitely. Doesn't care about timeout" +exclude :test_finds_pending_migrations, "Hangs indefinitely. Doesn't care about timeout" +exclude :test_migrator_db_has_no_schema_migrations_table, "Too slow, something's wrong." diff --git a/test/test_helper.rb b/test/test_helper.rb index 9621873b..758748ef 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -4,7 +4,8 @@ Bundler.require :development require "minitest/autorun" require "minitest/pride" -require "mocha/minitest" +require "minitest/excludes" + require "erb" require "byebug" if ENV["BYEBUG"] require "activerecord-postgis-adapter" @@ -65,7 +66,6 @@ class SpatialModel < ActiveRecord::Base end require 'timeout' -require 'stackprof' module TestTimeoutHelper def time_it From bbc38b46ddc01e4ab7c04f03cb50e9e1ff0b8e83 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 16:17:45 +0200 Subject: [PATCH 15/33] fix structure dump test --- .../ActiveRecord/PostgreSQLStructureDumpTest.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb diff --git a/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb b/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb new file mode 100644 index 00000000..398df340 --- /dev/null +++ b/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb @@ -0,0 +1,11 @@ + +setup do + @ignore_tables = ActiveRecord::SchemaDumper.ignore_tables + ActiveRecord::SchemaDumper.ignore_tables = [] + ARTest.config["connections"]["postgresql"] = { "arunit" => { "database" => ARTest.config["connections"]["postgis"]["arunit"]["database"] } } +end + +teardown do + ActiveRecord::SchemaDumper.ignore_tables = @ignore_tables + ARTest.config["connections"].delete("postgresql") +end From 9fb267cc75b167703b0458b4f7e6be1e64d4191b Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 16:26:32 +0200 Subject: [PATCH 16/33] add exclusions --- .../PoolConfig/ResolverTest.rb | 1 + .../Migration/ChangeSchemaTest.rb | 1 + .../Migration/CompatibilityTest.rb | 3 ++ .../Migration/InvalidOptionsTest.rb | 3 ++ .../Migration/PGChangeSchemaTest.rb | 1 + test/excludes/BasicsTest.rb | 1 + test/excludes/BulkAlterTableMigrationsTest.rb | 6 +++ test/excludes/CounterCacheTest.rb | 53 +++++++++++++++++++ test/excludes/DateTimePrecisionTest.rb | 1 + test/excludes/FinderTest.rb | 1 + test/excludes/MarshalSerializationTest.rb | 4 ++ test/excludes/MaterializedViewTest.rb | 1 + test/excludes/MultipleDbTest.rb | 1 + .../PostgresqlTimestampMigrationTest.rb | 2 + test/excludes/PostgresqlUUIDGenerationTest.rb | 1 + test/excludes/PreloaderTest.rb | 45 ++++++++++++++++ .../SameNameDifferentDatabaseFixturesTest.rb | 1 + test/excludes/SchemaDumperTest.rb | 2 + .../excludes/SchemaIndexIncludeColumnsTest.rb | 1 + test/excludes/UnsafeRawSqlTest.rb | 1 + test/excludes/ViewWithPrimaryKeyTest.rb | 1 + test/excludes/ViewWithoutPrimaryKeyTest.rb | 1 + test/test_helper.rb | 2 + 23 files changed, 134 insertions(+) create mode 100644 test/excludes/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/ChangeSchemaTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/CompatibilityTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/PGChangeSchemaTest.rb create mode 100644 test/excludes/BasicsTest.rb create mode 100644 test/excludes/BulkAlterTableMigrationsTest.rb create mode 100644 test/excludes/CounterCacheTest.rb create mode 100644 test/excludes/DateTimePrecisionTest.rb create mode 100644 test/excludes/FinderTest.rb create mode 100644 test/excludes/MarshalSerializationTest.rb create mode 100644 test/excludes/MaterializedViewTest.rb create mode 100644 test/excludes/MultipleDbTest.rb create mode 100644 test/excludes/PostgresqlTimestampMigrationTest.rb create mode 100644 test/excludes/PostgresqlUUIDGenerationTest.rb create mode 100644 test/excludes/PreloaderTest.rb create mode 100644 test/excludes/SameNameDifferentDatabaseFixturesTest.rb create mode 100644 test/excludes/SchemaDumperTest.rb create mode 100644 test/excludes/SchemaIndexIncludeColumnsTest.rb create mode 100644 test/excludes/UnsafeRawSqlTest.rb create mode 100644 test/excludes/ViewWithPrimaryKeyTest.rb create mode 100644 test/excludes/ViewWithoutPrimaryKeyTest.rb diff --git a/test/excludes/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb b/test/excludes/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb new file mode 100644 index 00000000..9b207ba7 --- /dev/null +++ b/test/excludes/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb @@ -0,0 +1 @@ +exclude "test_url_invalid_adapter", TRIAGE_MSG diff --git a/test/excludes/ActiveRecord/Migration/ChangeSchemaTest.rb b/test/excludes/ActiveRecord/Migration/ChangeSchemaTest.rb new file mode 100644 index 00000000..7f2a6fa7 --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/ChangeSchemaTest.rb @@ -0,0 +1 @@ +exclude "test_add_column_with_datetime_in_timestamptz_mode", TRIAGE_MSG diff --git a/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb new file mode 100644 index 00000000..ed61e2e0 --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb @@ -0,0 +1,3 @@ +exclude "test_legacy_change_column_with_null_executes_update", TRIAGE_MSG +exclude "test_disable_extension_on_7_0", TRIAGE_MSG +exclude "test_legacy_add_foreign_key_with_deferrable_true", TRIAGE_MSG diff --git a/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb b/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb new file mode 100644 index 00000000..b0c4d877 --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb @@ -0,0 +1,3 @@ +exclude "test_add_column_with_invalid_options", TRIAGE_MSG +exclude "test_add_reference_with_invalid_options", TRIAGE_MSG +exclude "test_change_column_with_invalid_options", TRIAGE_MSG diff --git a/test/excludes/ActiveRecord/Migration/PGChangeSchemaTest.rb b/test/excludes/ActiveRecord/Migration/PGChangeSchemaTest.rb new file mode 100644 index 00000000..5174b18f --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/PGChangeSchemaTest.rb @@ -0,0 +1 @@ +exclude "test_change_type_with_symbol_using_datetime_with_timestamptz_as_default", TRIAGE_MSG diff --git a/test/excludes/BasicsTest.rb b/test/excludes/BasicsTest.rb new file mode 100644 index 00000000..d4c83e18 --- /dev/null +++ b/test/excludes/BasicsTest.rb @@ -0,0 +1 @@ +exclude "test_column_names_are_escaped", TRIAGE_MSG diff --git a/test/excludes/BulkAlterTableMigrationsTest.rb b/test/excludes/BulkAlterTableMigrationsTest.rb new file mode 100644 index 00000000..c1c7b4b8 --- /dev/null +++ b/test/excludes/BulkAlterTableMigrationsTest.rb @@ -0,0 +1,6 @@ +exclude "test_changing_index", TRIAGE_MSG +exclude "test_changing_columns", TRIAGE_MSG +exclude "test_removing_index", TRIAGE_MSG +exclude "test_adding_indexes", TRIAGE_MSG +exclude "test_adding_multiple_columns", TRIAGE_MSG +exclude "test_changing_column_null_with_default", TRIAGE_MSG diff --git a/test/excludes/CounterCacheTest.rb b/test/excludes/CounterCacheTest.rb new file mode 100644 index 00000000..8c8ab7b5 --- /dev/null +++ b/test/excludes/CounterCacheTest.rb @@ -0,0 +1,53 @@ +exclude "test_update_multiple_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_reset_multiple_counters", TRIAGE_MSG +exclude "test_increment_counter", TRIAGE_MSG +exclude "test_decrement_counter_by_specific_amount", TRIAGE_MSG +exclude "test_update_counters_with_touch:_true", TRIAGE_MSG +exclude "test_reset_counter_performs_query_for_correct_counter_with_touch:_true", TRIAGE_MSG +exclude "test_reset_multiple_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_update_counter_for_decrement", TRIAGE_MSG +exclude "test_the_passed_symbol_needs_to_be_an_association_name_or_counter_name", TRIAGE_MSG +exclude "test_update_counter_for_decrement_for_cpk_model", TRIAGE_MSG +exclude "test_decrement_counter_for_cpk_model", TRIAGE_MSG +exclude "test_update_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_reset_counters_with_modularized_and_camelized_classnames", TRIAGE_MSG +exclude "test_decrement_counters_with_touch:_true", TRIAGE_MSG +exclude "test_increment_counters_with_touch:_true", TRIAGE_MSG +exclude "test_decrement_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_reset_multiple_counters_with_touch:_true", TRIAGE_MSG +exclude "test_reset_counters", TRIAGE_MSG +exclude "test_update_counters_in_a_polymorphic_relationship", TRIAGE_MSG +exclude "test_update_multiple_counters", TRIAGE_MSG +exclude "test_update_other_counters_on_parent_destroy", TRIAGE_MSG +exclude "test_reset_counters_with_string_argument", TRIAGE_MSG +exclude "test_reset_counter_with_belongs_to_which_has_class_name", TRIAGE_MSG +exclude "test_update_multiple_counters_with_touch:_true", TRIAGE_MSG +exclude "test_update_counters_of_multiple_records_with_touch:_true", TRIAGE_MSG +exclude "test_counter_caches_are_updated_in_memory_when_the_default_value_is_nil", TRIAGE_MSG +exclude "test_increment_counter_by_specific_amount", TRIAGE_MSG +exclude "test_decrement_counter", TRIAGE_MSG +exclude "test_reset_counters_with_touch:_true", TRIAGE_MSG +exclude "test_reset_the_right_counter_if_two_have_the_same_foreign_key", TRIAGE_MSG +exclude "test_inactive_conter_cache", TRIAGE_MSG +exclude "test_update_counters_of_multiple_records", TRIAGE_MSG +exclude "test_counters_are_updated_both_in_memory_and_in_the_database_on_create", TRIAGE_MSG +exclude "test_update_counter_with_initial_null_value", TRIAGE_MSG +exclude "test_decrement_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_increment_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_increment_counter_for_multiple_cpk_model_records", TRIAGE_MSG +exclude "test_reset_multiple_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_reset_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_reset_counters_by_counter_name", TRIAGE_MSG +exclude "test_reset_counter_skips_query_for_correct_counter", TRIAGE_MSG +exclude "test_increment_counter_for_cpk_model", TRIAGE_MSG +exclude "test_reset_counter_works_with_select_declared_on_association", TRIAGE_MSG +exclude "test_removing_association_updates_counter", TRIAGE_MSG +exclude "test_reset_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_reset_counters_for_cpk_model", TRIAGE_MSG +exclude "test_reset_the_right_counter_if_two_have_the_same_class_name", TRIAGE_MSG +exclude "test_active_conter_cache", TRIAGE_MSG +exclude "test_increment_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_update_multiple_counters_with_touch:_:written_on", TRIAGE_MSG +exclude "test_counter_cache_column?", TRIAGE_MSG +exclude "test_update_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG +exclude "test_reset_counter_of_has_many_:through_association", TRIAGE_MSG diff --git a/test/excludes/DateTimePrecisionTest.rb b/test/excludes/DateTimePrecisionTest.rb new file mode 100644 index 00000000..360661a3 --- /dev/null +++ b/test/excludes/DateTimePrecisionTest.rb @@ -0,0 +1 @@ +exclude "test_formatting_datetime_according_to_precision_when_time_zone_aware_using_timestamptz", TRIAGE_MSG diff --git a/test/excludes/FinderTest.rb b/test/excludes/FinderTest.rb new file mode 100644 index 00000000..e494cb9a --- /dev/null +++ b/test/excludes/FinderTest.rb @@ -0,0 +1 @@ +exclude "test_find_by_on_attribute_that_is_a_reserved_word", TRIAGE_MSG diff --git a/test/excludes/MarshalSerializationTest.rb b/test/excludes/MarshalSerializationTest.rb new file mode 100644 index 00000000..83e41c0c --- /dev/null +++ b/test/excludes/MarshalSerializationTest.rb @@ -0,0 +1,4 @@ +exclude "test_deserializing_rails_7_1_marshal_basic", TRIAGE_MSG +exclude "test_deserializing_rails_6_1_marshal_basic", TRIAGE_MSG +exclude "test_deserializing_rails_6_1_marshal_with_loaded_association_cache", TRIAGE_MSG +exclude "test_deserializing_rails_7_1_marshal_with_loaded_association_cache", TRIAGE_MSG diff --git a/test/excludes/MaterializedViewTest.rb b/test/excludes/MaterializedViewTest.rb new file mode 100644 index 00000000..64f72410 --- /dev/null +++ b/test/excludes/MaterializedViewTest.rb @@ -0,0 +1 @@ +exclude "test_views", TRIAGE_MSG diff --git a/test/excludes/MultipleDbTest.rb b/test/excludes/MultipleDbTest.rb new file mode 100644 index 00000000..4bc97de7 --- /dev/null +++ b/test/excludes/MultipleDbTest.rb @@ -0,0 +1 @@ +exclude "test_exception_contains_correct_pool", TRIAGE_MSG diff --git a/test/excludes/PostgresqlTimestampMigrationTest.rb b/test/excludes/PostgresqlTimestampMigrationTest.rb new file mode 100644 index 00000000..8b4fd2d8 --- /dev/null +++ b/test/excludes/PostgresqlTimestampMigrationTest.rb @@ -0,0 +1,2 @@ +exclude "test_adds_column_as_custom_type", TRIAGE_MSG +exclude "test_adds_column_as_timestamptz_if_datetime_type_changed", TRIAGE_MSG diff --git a/test/excludes/PostgresqlUUIDGenerationTest.rb b/test/excludes/PostgresqlUUIDGenerationTest.rb new file mode 100644 index 00000000..6faea968 --- /dev/null +++ b/test/excludes/PostgresqlUUIDGenerationTest.rb @@ -0,0 +1 @@ +exclude "test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration", TRIAGE_MSG diff --git a/test/excludes/PreloaderTest.rb b/test/excludes/PreloaderTest.rb new file mode 100644 index 00000000..7409c1d4 --- /dev/null +++ b/test/excludes/PreloaderTest.rb @@ -0,0 +1,45 @@ +exclude "test_preload_with_grouping_sets_inverse_association", TRIAGE_MSG +exclude "test_preload_wont_set_the_wrong_target", TRIAGE_MSG +exclude "test_preload_with_available_records_queries_when_incomplete", TRIAGE_MSG +exclude "test_preload_does_not_group_same_class_different_scope", TRIAGE_MSG +exclude "test_preloads_belongs_to_a_composite_primary_key_model_through_id_attribute", TRIAGE_MSG +exclude "test_preload_with_available_records_queries_when_scoped", TRIAGE_MSG +exclude "test_preload_with_instance_dependent_through_scope", TRIAGE_MSG +exclude "test_preload_grouped_queries_of_middle_records", TRIAGE_MSG +exclude "test_preload_groups_queries_with_same_scope", TRIAGE_MSG +exclude "test_preload_with_unpersisted_records_no_ops", TRIAGE_MSG +exclude "test_preload_makes_correct_number_of_queries_on_relation", TRIAGE_MSG +exclude "test_preload_keeps_built_belongs_to_records_no_ops", TRIAGE_MSG +exclude "test_preload_does_not_group_same_scope_different_key_name", TRIAGE_MSG +exclude "test_some_already_loaded_associations", TRIAGE_MSG +exclude "test_preload_grouped_queries_of_through_records", TRIAGE_MSG +exclude "test_preload_with_scope", TRIAGE_MSG +exclude "test_preload_groups_queries_with_same_scope_at_second_level", TRIAGE_MSG +exclude "test_preload_does_not_concatenate_duplicate_records", TRIAGE_MSG +exclude "test_preload_with_available_records_with_through_association", TRIAGE_MSG +exclude "test_preload_with_some_records_already_loaded", TRIAGE_MSG +exclude "test_preload_grouped_queries_with_already_loaded_records", TRIAGE_MSG +exclude "test_preload_with_through_instance_dependent_scope", TRIAGE_MSG +exclude "test_preload_can_group_separate_levels", TRIAGE_MSG +exclude "test_preload_with_instance_dependent_scope", TRIAGE_MSG +exclude "test_preload_loaded_belongs_to_association_with_composite_foreign_key", TRIAGE_MSG +exclude "test_preload_for_hmt_with_conditions", TRIAGE_MSG +exclude "test_preload_has_many_through_association_with_composite_query_constraints", TRIAGE_MSG +exclude "test_preload_with_only_some_records_available", TRIAGE_MSG +exclude "test_preload_has_many_association_with_composite_foreign_key", TRIAGE_MSG +exclude "test_preload_can_group_multi_level_ping_pong_through", TRIAGE_MSG +exclude "test_preload_with_available_records_sti", TRIAGE_MSG +exclude "test_preload_through_records_with_already_loaded_middle_record", TRIAGE_MSG +exclude "test_preload_with_available_records", TRIAGE_MSG +exclude "test_preload_makes_correct_number_of_queries_on_array", TRIAGE_MSG +exclude "test_preload_groups_queries_with_same_sql_at_second_level", TRIAGE_MSG +exclude "test_preload_through", TRIAGE_MSG +exclude "test_preload_keeps_built_has_many_records_after_query", TRIAGE_MSG +exclude "test_preload_with_available_records_with_multiple_classes", TRIAGE_MSG +exclude "test_preload_with_available_records_queries_when_collection", TRIAGE_MSG +exclude "test_preload_with_only_some_records_available_with_through_associations", TRIAGE_MSG +exclude "test_multi_database_polymorphic_preload_with_same_table_name", TRIAGE_MSG +exclude "test_preload_keeps_built_belongs_to_records_after_query", TRIAGE_MSG +exclude "test_preloads_has_many_on_model_with_a_composite_primary_key_through_id_attribute", TRIAGE_MSG +exclude "test_preload_belongs_to_association_with_composite_foreign_key", TRIAGE_MSG +exclude "test_preload_keeps_built_has_many_records_no_ops", TRIAGE_MSG diff --git a/test/excludes/SameNameDifferentDatabaseFixturesTest.rb b/test/excludes/SameNameDifferentDatabaseFixturesTest.rb new file mode 100644 index 00000000..f63f9c24 --- /dev/null +++ b/test/excludes/SameNameDifferentDatabaseFixturesTest.rb @@ -0,0 +1 @@ +exclude "test_fixtures_are_properly_loaded", TRIAGE_MSG diff --git a/test/excludes/SchemaDumperTest.rb b/test/excludes/SchemaDumperTest.rb new file mode 100644 index 00000000..fc09d7ab --- /dev/null +++ b/test/excludes/SchemaDumperTest.rb @@ -0,0 +1,2 @@ +exclude "test_schema_dump_with_timestamptz_datetime_format", TRIAGE_MSG +exclude "test_schema_dump_when_changing_datetime_type_for_an_existing_app", TRIAGE_MSG diff --git a/test/excludes/SchemaIndexIncludeColumnsTest.rb b/test/excludes/SchemaIndexIncludeColumnsTest.rb new file mode 100644 index 00000000..f7c836a8 --- /dev/null +++ b/test/excludes/SchemaIndexIncludeColumnsTest.rb @@ -0,0 +1 @@ +exclude "test_schema_dumps_index_included_columns", TRIAGE_MSG diff --git a/test/excludes/UnsafeRawSqlTest.rb b/test/excludes/UnsafeRawSqlTest.rb new file mode 100644 index 00000000..69be7044 --- /dev/null +++ b/test/excludes/UnsafeRawSqlTest.rb @@ -0,0 +1 @@ +exclude "test_order:_allows_valid_arguments_with_COLLATE", TRIAGE_MSG diff --git a/test/excludes/ViewWithPrimaryKeyTest.rb b/test/excludes/ViewWithPrimaryKeyTest.rb new file mode 100644 index 00000000..64f72410 --- /dev/null +++ b/test/excludes/ViewWithPrimaryKeyTest.rb @@ -0,0 +1 @@ +exclude "test_views", TRIAGE_MSG diff --git a/test/excludes/ViewWithoutPrimaryKeyTest.rb b/test/excludes/ViewWithoutPrimaryKeyTest.rb new file mode 100644 index 00000000..64f72410 --- /dev/null +++ b/test/excludes/ViewWithoutPrimaryKeyTest.rb @@ -0,0 +1 @@ +exclude "test_views", TRIAGE_MSG diff --git a/test/test_helper.rb b/test/test_helper.rb index 758748ef..f70ab906 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,6 +10,8 @@ require "byebug" if ENV["BYEBUG"] require "activerecord-postgis-adapter" +TRIAGE_MSG = "Needs triage and fixes. See #378" + if ENV["ARCONN"] # only install activerecord schema if we need it require "cases/helper" From 69012e33a91646e850435728c92094b6b320e35e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 5 Sep 2024 17:13:00 +0200 Subject: [PATCH 17/33] cleanup --- .github/workflows/tests.yml | 2 +- Gemfile | 4 ++++ test/cases/ddl_test.rb | 15 +++++++++------ test/excludes/CounterCacheTest.rb | 2 ++ test/test_helper.rb | 2 +- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6fe5b0bc..cd285614 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -72,4 +72,4 @@ jobs: PGHOST: 127.0.0.1 PGUSER: postgres PGPASSWORD: postgres - TESTOPTS: --verbose --profile=3 + TESTOPTS: --profile=3 diff --git a/Gemfile b/Gemfile index 7fe2ecb8..f4783c70 100644 --- a/Gemfile +++ b/Gemfile @@ -38,4 +38,8 @@ group :development do gem "bcrypt" gem "sqlite3" gem "msgpack" + + # Still used a little bit in our tests. + # TODO: get rid of the dependency + gem "mocha" end diff --git a/test/cases/ddl_test.rb b/test/cases/ddl_test.rb index 94890555..36301b7f 100644 --- a/test/cases/ddl_test.rb +++ b/test/cases/ddl_test.rb @@ -3,7 +3,7 @@ require_relative "../test_helper" module PostGIS - class DDLTest < ActiveSupport::TestCase + class DDLTest < ActiveRecord::TestCase def test_spatial_column_options [ :geography, @@ -279,11 +279,14 @@ def test_no_query_spatial_column_info t.string "name" end klass.reset_column_information - # `all` queries column info from the database - it should not be called when klass.columns is called - ActiveRecord::ConnectionAdapters::PostGIS::SpatialColumnInfo.any_instance.expects(:all).never - # first column is id, second is name - refute klass.columns[1].spatial? - assert_nil klass.columns[1].has_z + + # `SpatialColumnInfo#all` queries column info from the database. + # It should not be called when klass.columns is called + assert_queries_count(0) do + # first column is id, second is name + refute klass.columns[1].spatial? + assert_nil klass.columns[1].has_z + end end # Ensure that null contraints info is getting captured like the diff --git a/test/excludes/CounterCacheTest.rb b/test/excludes/CounterCacheTest.rb index 8c8ab7b5..6082c131 100644 --- a/test/excludes/CounterCacheTest.rb +++ b/test/excludes/CounterCacheTest.rb @@ -3,7 +3,9 @@ exclude "test_increment_counter", TRIAGE_MSG exclude "test_decrement_counter_by_specific_amount", TRIAGE_MSG exclude "test_update_counters_with_touch:_true", TRIAGE_MSG +exclude "test_update_counters_doesn't_touch_timestamps_by_default", TRIAGE_MSG exclude "test_reset_counter_performs_query_for_correct_counter_with_touch:_true", TRIAGE_MSG +exclude "test_update_counters_doesn't_touch_timestamps_with_touch:_[]", TRIAGE_MSG exclude "test_reset_multiple_counters_with_touch:_%i(_updated_at_written_on_)", TRIAGE_MSG exclude "test_update_counter_for_decrement", TRIAGE_MSG exclude "test_the_passed_symbol_needs_to_be_an_association_name_or_counter_name", TRIAGE_MSG diff --git a/test/test_helper.rb b/test/test_helper.rb index f70ab906..37a8ad86 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,7 +82,7 @@ def time_it end end -module ActiveRecord +module ActiveSupport class TestCase include TestTimeoutHelper From ada1f969d0d3e33255ec84d70f1a48d26cf71cc3 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 9 Sep 2024 09:04:36 +0200 Subject: [PATCH 18/33] longer timeout --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index cd285614..98fdad07 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -73,3 +73,4 @@ jobs: PGUSER: postgres PGPASSWORD: postgres TESTOPTS: --profile=3 + TEST_TIMEOUT: 30 From 0064eeae00953ba06d0695e251bf85e3bed7793e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 9 Sep 2024 09:16:43 +0200 Subject: [PATCH 19/33] more excludes --- test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb | 2 ++ test/excludes/FixturesResetPkSequenceTest.rb | 1 + 2 files changed, 3 insertions(+) create mode 100644 test/excludes/FixturesResetPkSequenceTest.rb diff --git a/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb b/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb index 398df340..ff5b71bc 100644 --- a/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb +++ b/test/excludes/ActiveRecord/PostgreSQLStructureDumpTest.rb @@ -9,3 +9,5 @@ ActiveRecord::SchemaDumper.ignore_tables = @ignore_tables ARTest.config["connections"].delete("postgresql") end + +exclude :test_structure_dump, TRIAGE_MSG + " / Likely due to CI installation of pg_dump vs postges" diff --git a/test/excludes/FixturesResetPkSequenceTest.rb b/test/excludes/FixturesResetPkSequenceTest.rb new file mode 100644 index 00000000..bcffd9fc --- /dev/null +++ b/test/excludes/FixturesResetPkSequenceTest.rb @@ -0,0 +1 @@ +exclude :test_resets_to_min_pk_with_specified_pk_and_sequence, TRIAGE_MSG From 9dab4f51232083c83a1d1cb2d07a093d52bdd8c1 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 9 Sep 2024 09:39:34 +0200 Subject: [PATCH 20/33] more excludes --- test/excludes/PostgresqlInvertibleMigrationTest.rb | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/excludes/PostgresqlInvertibleMigrationTest.rb diff --git a/test/excludes/PostgresqlInvertibleMigrationTest.rb b/test/excludes/PostgresqlInvertibleMigrationTest.rb new file mode 100644 index 00000000..d6fe94fd --- /dev/null +++ b/test/excludes/PostgresqlInvertibleMigrationTest.rb @@ -0,0 +1 @@ +exclude :test_migrate_revert_add_and_validate_foreign_key, TRIAGE_MSG + " / flaky" From eea4b7662870dd7f47c813777cd1c08722424c33 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 9 Sep 2024 09:51:25 +0200 Subject: [PATCH 21/33] fix flaky tests --- test/cases/basic_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/cases/basic_test.rb b/test/cases/basic_test.rb index b86b80fc..f3546e6a 100644 --- a/test/cases/basic_test.rb +++ b/test/cases/basic_test.rb @@ -116,6 +116,7 @@ def test_custom_factory end klass.reset_column_information custom_factory = RGeo::Geographic.spherical_factory(buffer_resolution: 8, srid: 4326) + old_registry = spatial_factory_store.registry spatial_factory_store.register(custom_factory, geo_type: "polygon", srid: 4326) object = klass.new area = custom_factory.point(1, 2).buffer(3) @@ -123,6 +124,8 @@ def test_custom_factory object.save! object.reload assert_equal area, object.area + ensure + spatial_factory_store.registry = old_registry end def test_spatial_factory_attrs_parsing @@ -132,12 +135,14 @@ def test_spatial_factory_attrs_parsing end klass.reset_column_information factory = RGeo::Cartesian.preferred_factory(srid: 4326) + old_registry = spatial_factory_store.registry spatial_factory_store.register(factory, { srid: 4326, sql_type: "geometry", geo_type: "multi_polygon", has_z: false, has_m: false }) # wrong factory for default + old_default = spatial_factory_store.default spatial_factory_store.default = RGeo::Geographic.spherical_factory(srid: 4326) object = klass.new @@ -145,10 +150,14 @@ def test_spatial_factory_attrs_parsing object.save! object.reload assert_equal(factory, object.areas.factory) + ensure + spatial_factory_store.registry = old_registry + spatial_factory_store.default = old_default end def test_readme_example geo_factory = RGeo::Geographic.spherical_factory(srid: 4326) + old_registry = spatial_factory_store.registry spatial_factory_store.register(geo_factory, geo_type: "point", sql_type: "geography") klass = SpatialModel @@ -173,6 +182,8 @@ def test_readme_example object.save! object.reload refute_equal geo_factory, object.shape.factory + ensure + spatial_factory_store.registry = old_registry end def test_point_to_json From 763cef8c49f545f33f827f2b403d45fa761627fe Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Sun, 15 Sep 2024 18:54:09 +0200 Subject: [PATCH 22/33] bump rgeo-activerecord --- activerecord-postgis-adapter.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord-postgis-adapter.gemspec b/activerecord-postgis-adapter.gemspec index 0378c05c..1fef87d2 100644 --- a/activerecord-postgis-adapter.gemspec +++ b/activerecord-postgis-adapter.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.1.0" spec.add_dependency "activerecord", "~> 7.2.0" - spec.add_dependency "rgeo-activerecord", "~> 7.0.0" + spec.add_dependency "rgeo-activerecord", "~> 8.0.0" spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "minitest", "~> 5.4" From acbe7c8fde12f64d9662e3b09df230e3481152df Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 17:40:18 +0200 Subject: [PATCH 23/33] debug info --- .../connection_adapters/postgis/oid/spatial.rb | 12 ++++++++++++ test/test_helper.rb | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/active_record/connection_adapters/postgis/oid/spatial.rb b/lib/active_record/connection_adapters/postgis/oid/spatial.rb index 46ed619f..68e19d0b 100644 --- a/lib/active_record/connection_adapters/postgis/oid/spatial.rb +++ b/lib/active_record/connection_adapters/postgis/oid/spatial.rb @@ -71,6 +71,18 @@ def type def serialize(value) return if value.nil? geo_value = cast_value(value) + if spatial_factory.srid != @srid + $something_wrong = true + puts ?* * 100 + puts "@geo_type: #{@geo_type}" + puts "type.to_s: #{type.to_s}" + puts "@srid: #{@srid}" + p spatial_factory # It has srid=0 when bugged + p value + p geo_value + puts + puts + end # TODO - only valid types should be allowed # e.g. linestring is not valid for point column diff --git a/test/test_helper.rb b/test/test_helper.rb index 37a8ad86..ffdaa08c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,10 +82,23 @@ def time_it end end +$something_wrong = false +$last_test = nil module ActiveSupport class TestCase include TestTimeoutHelper + teardown do + if $something_wrong + puts + puts "SOMETHING WRONG in test #{name.inspect}" + puts "ran after #{$last_test.inspect}" + puts + $something_wrong = false + end + $last_test = name + end + def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) end From 485ccb5deff154992eb53dc88189f9c62b9ca013 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 18:42:03 +0200 Subject: [PATCH 24/33] fixing maybe --- test/cases/basic_test.rb | 4 ++++ test/test_helper.rb | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/test/cases/basic_test.rb b/test/cases/basic_test.rb index f3546e6a..9ee0dd93 100644 --- a/test/cases/basic_test.rb +++ b/test/cases/basic_test.rb @@ -8,6 +8,10 @@ def before reset_spatial_store end + def after + reset_spatial_store + end + def test_version refute_nil ActiveRecord::ConnectionAdapters::PostGIS::VERSION end diff --git a/test/test_helper.rb b/test/test_helper.rb index ffdaa08c..aa347ead 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -107,6 +107,11 @@ def geographic_factory RGeo::Geographic.spherical_factory(srid: 4326) end + # TODO: rather than using this, we should somehow make this a + # fixture that has its self-handled lifecycle. Right now we + # are depending on the dev running `reset_spatial_store` if + # they made any update. They can forget it (with the current + # flacky tests we have, it seems that is actually the case). def spatial_factory_store RGeo::ActiveRecord::SpatialFactoryStore.instance end From 7d58504d16da3e947190dddb5494c3a8eebf0367 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 18:51:51 +0200 Subject: [PATCH 25/33] more debug info --- lib/active_record/connection_adapters/postgis/oid/spatial.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/active_record/connection_adapters/postgis/oid/spatial.rb b/lib/active_record/connection_adapters/postgis/oid/spatial.rb index 68e19d0b..7de92a50 100644 --- a/lib/active_record/connection_adapters/postgis/oid/spatial.rb +++ b/lib/active_record/connection_adapters/postgis/oid/spatial.rb @@ -53,6 +53,7 @@ def self.parse_sql_type(sql_type) end def spatial_factory + pp factory_attrs @spatial_factory ||= RGeo::ActiveRecord::SpatialFactoryStore.instance.factory( factory_attrs From 7b13ba679f7637d8dfdf9bd768d8549f3f407bd9 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 19:00:34 +0200 Subject: [PATCH 26/33] intent tracer usage --- Gemfile | 1 + .../connection_adapters/postgis/oid/spatial.rb | 14 ++++++++++++-- test/cases/attributes_test.rb | 1 - 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index f4783c70..7abffa3b 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,7 @@ gemspec gem "pg", "~> 1.0", platform: :ruby gem "byebug" if ENV["BYEBUG"] +gem "tracer" def activerecord_version return ENV["AR_VERSION"] if ENV["AR_VERSION"] diff --git a/lib/active_record/connection_adapters/postgis/oid/spatial.rb b/lib/active_record/connection_adapters/postgis/oid/spatial.rb index 7de92a50..c369c2a2 100644 --- a/lib/active_record/connection_adapters/postgis/oid/spatial.rb +++ b/lib/active_record/connection_adapters/postgis/oid/spatial.rb @@ -53,11 +53,21 @@ def self.parse_sql_type(sql_type) end def spatial_factory - pp factory_attrs - @spatial_factory ||= + return @spatial_factory if defined?(@spatial_factory) + @spatial_factory = RGeo::ActiveRecord::SpatialFactoryStore.instance.factory( factory_attrs ) + + if @srid != @spatial_factory.srid + puts + puts "INITIALIZATION ERROR" + puts + end + + require "tracer" + Tracer.trace(@spatial_factory) + @spatial_factory end def spatial? diff --git a/test/cases/attributes_test.rb b/test/cases/attributes_test.rb index d1642e95..ade9e98e 100644 --- a/test/cases/attributes_test.rb +++ b/test/cases/attributes_test.rb @@ -23,7 +23,6 @@ class InvalidAttribute < ActiveRecord::Base class AttributesTest < ActiveSupport::TestCase def setup - reset_spatial_store create_foo create_spatial_foo create_invalid_attributes From 42e8e55578df01428efd6d80911d63f233e3c851 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 19:09:21 +0200 Subject: [PATCH 27/33] no tracer --- lib/active_record/connection_adapters/postgis/oid/spatial.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/postgis/oid/spatial.rb b/lib/active_record/connection_adapters/postgis/oid/spatial.rb index c369c2a2..da23d42f 100644 --- a/lib/active_record/connection_adapters/postgis/oid/spatial.rb +++ b/lib/active_record/connection_adapters/postgis/oid/spatial.rb @@ -65,8 +65,9 @@ def spatial_factory puts end - require "tracer" - Tracer.trace(@spatial_factory) + # Tracer is waaay too noisy + # require "tracer" + # Tracer.trace(@spatial_factory) @spatial_factory end From 956e5f8da0f0f7b995b04606eff03add88d9020c Mon Sep 17 00:00:00 2001 From: Rafael Santos Date: Wed, 16 Oct 2024 20:50:45 +1300 Subject: [PATCH 28/33] Fix tests of #405 "Bump to 7.2.0" (#412) * load current default value as is instead of making rgeo produce one. produced default is not memoized by rgeo. * fix rubocop layout warning on basic_test.rb * avoid using whole ar-testcase which causes trouble when running test:postgis * ensure connection is stablished test was failing when run in isolation --- test/cases/basic_test.rb | 5 +++-- test/cases/ddl_test.rb | 5 ++++- test/cases/schema_statements_test.rb | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/test/cases/basic_test.rb b/test/cases/basic_test.rb index 9ee0dd93..608170c4 100644 --- a/test/cases/basic_test.rb +++ b/test/cases/basic_test.rb @@ -18,7 +18,8 @@ def test_version def test_postgis_available assert_equal "PostGIS", SpatialModel.lease_connection.adapter_name - assert_equal SpatialModel.lease_connection.select_value("SELECT postgis_lib_version()"), SpatialModel.lease_connection.postgis_lib_version + expected_postgis_lib_version_value = SpatialModel.lease_connection.select_value("SELECT postgis_lib_version()") + assert_equal expected_postgis_lib_version_value, SpatialModel.lease_connection.postgis_lib_version valid_version = ["2.", "3."].any? { |major_ver| SpatialModel.lease_connection.postgis_lib_version.start_with? major_ver } assert valid_version end @@ -146,7 +147,7 @@ def test_spatial_factory_attrs_parsing has_z: false, has_m: false }) # wrong factory for default - old_default = spatial_factory_store.default + old_default = spatial_factory_store.instance_variable_get :@default spatial_factory_store.default = RGeo::Geographic.spherical_factory(srid: 4326) object = klass.new diff --git a/test/cases/ddl_test.rb b/test/cases/ddl_test.rb index 36301b7f..0b647bda 100644 --- a/test/cases/ddl_test.rb +++ b/test/cases/ddl_test.rb @@ -1,9 +1,12 @@ # frozen_string_literal: true require_relative "../test_helper" +require "active_record/testing/query_assertions" module PostGIS - class DDLTest < ActiveRecord::TestCase + class DDLTest < ActiveSupport::TestCase + include ActiveRecord::Assertions::QueryAssertions + def test_spatial_column_options [ :geography, diff --git a/test/cases/schema_statements_test.rb b/test/cases/schema_statements_test.rb index 640f7650..89a0d48c 100644 --- a/test/cases/schema_statements_test.rb +++ b/test/cases/schema_statements_test.rb @@ -6,7 +6,7 @@ module PostGIS class SchemaStatementsTest < ActiveSupport::TestCase def test_initialize_type_map SpatialModel.with_connection do |connection| - assert connection.connected? + connection.connect! initialized_types = connection.send(:type_map).keys # PostGIS types must be initialized first, so From 4f6ebe68915bf3c4c9daed61c7faa57d0a4d7ef5 Mon Sep 17 00:00:00 2001 From: Rafael Santos Date: Thu, 17 Oct 2024 20:57:04 +1300 Subject: [PATCH 29/33] fix test teardown (#413) --- test/cases/basic_test.rb | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/test/cases/basic_test.rb b/test/cases/basic_test.rb index 608170c4..f8fe4bff 100644 --- a/test/cases/basic_test.rb +++ b/test/cases/basic_test.rb @@ -4,11 +4,7 @@ module PostGIS class BasicTest < ActiveSupport::TestCase - def before - reset_spatial_store - end - - def after + def teardown reset_spatial_store end @@ -121,7 +117,6 @@ def test_custom_factory end klass.reset_column_information custom_factory = RGeo::Geographic.spherical_factory(buffer_resolution: 8, srid: 4326) - old_registry = spatial_factory_store.registry spatial_factory_store.register(custom_factory, geo_type: "polygon", srid: 4326) object = klass.new area = custom_factory.point(1, 2).buffer(3) @@ -129,8 +124,6 @@ def test_custom_factory object.save! object.reload assert_equal area, object.area - ensure - spatial_factory_store.registry = old_registry end def test_spatial_factory_attrs_parsing @@ -140,14 +133,12 @@ def test_spatial_factory_attrs_parsing end klass.reset_column_information factory = RGeo::Cartesian.preferred_factory(srid: 4326) - old_registry = spatial_factory_store.registry spatial_factory_store.register(factory, { srid: 4326, sql_type: "geometry", geo_type: "multi_polygon", has_z: false, has_m: false }) # wrong factory for default - old_default = spatial_factory_store.instance_variable_get :@default spatial_factory_store.default = RGeo::Geographic.spherical_factory(srid: 4326) object = klass.new @@ -155,14 +146,10 @@ def test_spatial_factory_attrs_parsing object.save! object.reload assert_equal(factory, object.areas.factory) - ensure - spatial_factory_store.registry = old_registry - spatial_factory_store.default = old_default end def test_readme_example geo_factory = RGeo::Geographic.spherical_factory(srid: 4326) - old_registry = spatial_factory_store.registry spatial_factory_store.register(geo_factory, geo_type: "point", sql_type: "geography") klass = SpatialModel @@ -187,8 +174,6 @@ def test_readme_example object.save! object.reload refute_equal geo_factory, object.shape.factory - ensure - spatial_factory_store.registry = old_registry end def test_point_to_json From 04dd5278c4ef60dd8c392f32daaad6f777c5a21a Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 17 Oct 2024 09:59:54 +0200 Subject: [PATCH 30/33] typos Co-authored-by: Geremia Taglialatela --- test/cases/ddl_test.rb | 2 +- test/test_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cases/ddl_test.rb b/test/cases/ddl_test.rb index 0b647bda..c4936eef 100644 --- a/test/cases/ddl_test.rb +++ b/test/cases/ddl_test.rb @@ -292,7 +292,7 @@ def test_no_query_spatial_column_info end end - # Ensure that null contraints info is getting captured like the + # Ensure that null constraints info is getting captured like the # normal adapter. def test_null_constraints klass.lease_connection.create_table(:spatial_models, force: true) do |t| diff --git a/test/test_helper.rb b/test/test_helper.rb index aa347ead..90481014 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -111,7 +111,7 @@ def geographic_factory # fixture that has its self-handled lifecycle. Right now we # are depending on the dev running `reset_spatial_store` if # they made any update. They can forget it (with the current - # flacky tests we have, it seems that is actually the case). + # flaky tests we have, it seems that is actually the case). def spatial_factory_store RGeo::ActiveRecord::SpatialFactoryStore.instance end From ac74f4ee8db348de6ea9eb29f05a04791d1cceb1 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 7 Oct 2024 19:03:10 +0200 Subject: [PATCH 31/33] chore: add fundings --- .github/FUNDING.yml | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/FUNDING.yml diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 00000000..91cdd4ff --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1 @@ +open_collective: rgeo From 435027dec04898d9e2f840652f1e19b236e55f8e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 17 Oct 2024 10:34:00 +0200 Subject: [PATCH 32/33] cleanup --- .github/workflows/tests.yml | 2 +- Gemfile | 7 ----- activerecord-postgis-adapter.gemspec | 4 ++- .../postgis/oid/spatial.rb | 26 +------------------ test/cases/ddl_test.rb | 2 +- test/rake_helper.rb | 2 +- test/test_helper.rb | 13 ---------- 7 files changed, 7 insertions(+), 49 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 98fdad07..b9c5e80d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -69,7 +69,7 @@ jobs: - name: Run Tests run: bundle exec rake test env: - PGHOST: 127.0.0.1 + PGHOST: localhost PGUSER: postgres PGPASSWORD: postgres TESTOPTS: --profile=3 diff --git a/Gemfile b/Gemfile index 7abffa3b..5976f205 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,6 @@ gemspec gem "pg", "~> 1.0", platform: :ruby gem "byebug" if ENV["BYEBUG"] -gem "tracer" def activerecord_version return ENV["AR_VERSION"] if ENV["AR_VERSION"] @@ -33,14 +32,8 @@ end gem "rails", github: "rails/rails", tag: "v#{activerecord_version}" group :development do - gem "minitest-excludes", "~> 2.0" - # Gems used by the ActiveRecord test suite gem "bcrypt" gem "sqlite3" gem "msgpack" - - # Still used a little bit in our tests. - # TODO: get rid of the dependency - gem "mocha" end diff --git a/activerecord-postgis-adapter.gemspec b/activerecord-postgis-adapter.gemspec index 1fef87d2..c256c562 100644 --- a/activerecord-postgis-adapter.gemspec +++ b/activerecord-postgis-adapter.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |spec| spec.files = Dir["lib/**/*", "LICENSE.txt"] spec.platform = Gem::Platform::RUBY - # # ruby-lang.org/en/downloads/branches + # ruby-lang.org/en/downloads/branches spec.required_ruby_version = ">= 3.1.0" spec.add_dependency "activerecord", "~> 7.2.0" @@ -25,10 +25,12 @@ Gem::Specification.new do |spec| spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "minitest", "~> 5.4" + spec.add_development_dependency "minitest-excludes", "~> 2.0" spec.add_development_dependency "benchmark-ips", "~> 2.12" spec.add_development_dependency "rubocop", "~> 1.50" spec.metadata = { + "funding_uri" => "https://opencollective.com/rgeo", "rubygems_mfa_required" => "true" } end diff --git a/lib/active_record/connection_adapters/postgis/oid/spatial.rb b/lib/active_record/connection_adapters/postgis/oid/spatial.rb index da23d42f..46ed619f 100644 --- a/lib/active_record/connection_adapters/postgis/oid/spatial.rb +++ b/lib/active_record/connection_adapters/postgis/oid/spatial.rb @@ -53,22 +53,10 @@ def self.parse_sql_type(sql_type) end def spatial_factory - return @spatial_factory if defined?(@spatial_factory) - @spatial_factory = + @spatial_factory ||= RGeo::ActiveRecord::SpatialFactoryStore.instance.factory( factory_attrs ) - - if @srid != @spatial_factory.srid - puts - puts "INITIALIZATION ERROR" - puts - end - - # Tracer is waaay too noisy - # require "tracer" - # Tracer.trace(@spatial_factory) - @spatial_factory end def spatial? @@ -83,18 +71,6 @@ def type def serialize(value) return if value.nil? geo_value = cast_value(value) - if spatial_factory.srid != @srid - $something_wrong = true - puts ?* * 100 - puts "@geo_type: #{@geo_type}" - puts "type.to_s: #{type.to_s}" - puts "@srid: #{@srid}" - p spatial_factory # It has srid=0 when bugged - p value - p geo_value - puts - puts - end # TODO - only valid types should be allowed # e.g. linestring is not valid for point column diff --git a/test/cases/ddl_test.rb b/test/cases/ddl_test.rb index c4936eef..86a16e29 100644 --- a/test/cases/ddl_test.rb +++ b/test/cases/ddl_test.rb @@ -285,7 +285,7 @@ def test_no_query_spatial_column_info # `SpatialColumnInfo#all` queries column info from the database. # It should not be called when klass.columns is called - assert_queries_count(0) do + assert_no_queries do # first column is id, second is name refute klass.columns[1].spatial? assert_nil klass.columns[1].has_z diff --git a/test/rake_helper.rb b/test/rake_helper.rb index 8819c6d4..adaa7236 100644 --- a/test/rake_helper.rb +++ b/test/rake_helper.rb @@ -29,7 +29,7 @@ def activerecord_test_files .map { |file| File.join ar_root, file.strip } .sort .prepend(POSTGIS_TEST_HELPER) - .then{ FileList[*_1] } + .then { FileList[*_1] } else FileList["#{ar_root}/test/cases/**/*_test.rb"] .reject { _1.include?("/adapters/") || _1.include?("/encryption/performance") } diff --git a/test/test_helper.rb b/test/test_helper.rb index 90481014..1a001c38 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,23 +82,10 @@ def time_it end end -$something_wrong = false -$last_test = nil module ActiveSupport class TestCase include TestTimeoutHelper - teardown do - if $something_wrong - puts - puts "SOMETHING WRONG in test #{name.inspect}" - puts "ran after #{$last_test.inspect}" - puts - $something_wrong = false - end - $last_test = name - end - def factory(srid: 3785) RGeo::Cartesian.preferred_factory(srid: srid) end From ecf8ef317e94238faf039f220d2be85aca7575f5 Mon Sep 17 00:00:00 2001 From: Keith Doggett Date: Mon, 4 Nov 2024 15:25:20 -0500 Subject: [PATCH 33/33] Bump version, update history and README --- History.md | 6 +++++- README.md | 8 ++++++++ lib/active_record/connection_adapters/postgis/version.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index e419df75..cd05a927 100644 --- a/History.md +++ b/History.md @@ -1,4 +1,8 @@ -### 9.0.2 / 2024-04-39 +### 10.0.0 / 2024-11-04 + +* ActiveRecord 7.2 support #405 + +### 9.0.2 / 2024-04-30 * Add `ConnectionHandling` module (copiousfreetime) #390 diff --git a/README.md b/README.md index 12d48eb2..8643989e 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,14 @@ Gemfile: gem 'activerecord-postgis-adapter' ``` +#### Version 10.x supports ActiveRecord 7.2 + +``` +ActiveRecord 7.2 +Ruby 3.1.0+ +PostGIS 2.0+ +``` + #### Version 9.x supports ActiveRecord 7.1 ``` diff --git a/lib/active_record/connection_adapters/postgis/version.rb b/lib/active_record/connection_adapters/postgis/version.rb index 113f10b7..65aebb0e 100644 --- a/lib/active_record/connection_adapters/postgis/version.rb +++ b/lib/active_record/connection_adapters/postgis/version.rb @@ -3,7 +3,7 @@ module ActiveRecord module ConnectionAdapters module PostGIS - VERSION = "9.0.2" + VERSION = "10.0.0" end end end