Bump to 7.2.0 #405

BuonOmo · 2024-09-04T12:10:02Z

Closes #403
Closes #395

activerecord-postgis-adapter.gemspec

BuonOmo · 2024-09-04T12:45:55Z

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"


This is the only actual needed change according to the current test suite, but I'm trying with the whole rails test suite to be sure we do not miss anything

seuros · 2024-09-04T12:57:16Z

Rakefile

@@ -3,7 +3,7 @@ require "rake/testtask"
 require_relative "test/rake_helper"

 task default: [:test]
-task test: "test:postgis"
+task test: "test:all"


i think the whole suite is flaky.

I'll look at it. I have good experience with this suite now that I maintain the CRDB adapter. It usually is flaky due to some tests modifying the db. Which they should not do if they are correctly changed for our use case. Do you have any further information?

Just this : #378

Thx, I plan on excluding using minitest-exclude and opening an issue for excluded tests

@BuonOmo yeah I hadn't figured out how to replicate the exclusions like in the CRDB adapter, but we definitely need that here since some are failing (which is expected). But to @seuros 's point that's why I just left the postgis tests for now.

Now we only have Postgis basic tests flaky. If one of you has time to look into it it would be much appreciated. Otherwise maybe we can skip them for now and make this or top priority issue, just so we can release this ?

@BuonOmo so just to make sure I'm clear, there's a few postgis tests that were working and are now flakey? I can try to work through some of them. Is it only in the CI?

I noticed some of the tests failures are order related.
notes here: #405 (comment)

activerecord-postgis-adapter.gemspec

seuros · 2024-09-04T13:56:29Z

test/cases/attributes_test.rb

@@ -99,20 +99,20 @@ 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|


these test case work in isolation

What do you mean?

I mean if you run any test that break alone, it works.

Something wrong with CI that we should fix.
Checkout https://github.com/nektos/act if you want to have your own github runner locally.

Thanks for act, I am still unsure about your first point though..

formigarafa · 2024-10-13T22:44:41Z

I've got to something more specific:
BasicTest#test_save_and_load_point fails when it runs after BasicTest#test_spatial_factory_attrs_parsing.

formigarafa · 2024-10-14T03:22:37Z

Please have a look at #412 . Existing tests are passing, now.

* 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

tagliala

Hello, thanks for this work here

I randomly ran into a couple of typos, one is not related to this PR but there was a change close enough

test/cases/ddl_test.rb

test/test_helper.rb

Co-authored-by: Geremia Taglialatela <tagliala@users.noreply.github.com>

seuros

Should we keep the excludes files ?

BuonOmo · 2024-10-17T12:42:55Z

@seuros I'm not sure I get you. Yes we should keep the files I added, they correspond to failing tests! And let's track them in the related issue #378.

formigarafa · 2024-10-25T00:55:13Z

About these ignore files: are they working as temporary to reduce the noise while this work is done (like a skip), are they mostly definitive like for unnecessary tests or tests that do not apply in the context of PostGIS or are they a mix of both?

I am asking because I've made a few adjustments to make the ignored test appear as skipped to have an idea of the size of the problem and I wouldn't mind sharing that if it appears useful to get over of what is missing.

BuonOmo · 2024-10-25T09:08:58Z

@formigarafa I think they would be a mix of both. I haven't checked them individually besides just seeing that they were failing.

I am asking because I've made a few adjustments to make the ignored test appear as skipped to have an idea of the size of the problem and I wouldn't mind sharing that if it appears useful to get over of what is missing.

This looks like a change for the gem responsible of ignoring tests ? Anyway, feel free to submit this!

keithdoggett

Thank you for the work on this everyone! @BuonOmo would you like me to handle creating a stable branch for the old version and bumping/releasing this?

BuonOmo · 2024-11-01T09:33:33Z

@keithdoggett lets go! Thank you :)

I'll take a look at the new PR #415 soon, but I guess we could already push this, and have a patch release for rake task fix

keithdoggett · 2024-11-04T20:32:58Z

v10.0.0 is released. 9.0-stable branch also up if anyone wants to build directly from git for AR7.1. Thanks everyone!

feat: bump activerecord

a62542d

BuonOmo marked this pull request as ready for review September 4, 2024 12:10

BuonOmo force-pushed the bump-7-2 branch 4 times, most recently from 8d66e71 to 0ac2822 Compare September 4, 2024 12:19

feat: enable all tests

c21dbce

BuonOmo force-pushed the bump-7-2 branch from 0ac2822 to c21dbce Compare September 4, 2024 12:25

BuonOmo changed the title ~~Bump 7 2~~ Bump to 7.2.0 Sep 4, 2024

BuonOmo commented Sep 4, 2024

View reviewed changes

activerecord-postgis-adapter.gemspec Outdated Show resolved Hide resolved

BuonOmo commented Sep 4, 2024

View reviewed changes

seuros reviewed Sep 4, 2024

View reviewed changes

BuonOmo force-pushed the bump-7-2 branch from b813654 to aae145c Compare September 4, 2024 13:26

seuros reviewed Sep 4, 2024

View reviewed changes

BuonOmo force-pushed the bump-7-2 branch from aae145c to 14d81ff Compare September 4, 2024 14:32

fix: use lease_connection in tests

28cab3a

BuonOmo force-pushed the bump-7-2 branch from 14d81ff to 28cab3a Compare September 5, 2024 06:43

BuonOmo added 2 commits September 5, 2024 09:33

add tracer to figure it out

b4ed5bf

stackprof less verbose

bc31a28

BuonOmo force-pushed the bump-7-2 branch from f8b207a to afeab02 Compare September 5, 2024 09:25

enable_ext analysis

5fd9f6c

BuonOmo force-pushed the bump-7-2 branch from afeab02 to 5fd9f6c Compare September 5, 2024 09:31

enable_ext stackprof

3be96f9

BuonOmo force-pushed the bump-7-2 branch from 92e76f0 to 3be96f9 Compare September 5, 2024 09:57

BuonOmo added 2 commits September 5, 2024 12:19

show conninfo

eee50f6

debug conninfo

ec1c5ef

BuonOmo force-pushed the bump-7-2 branch from 1928703 to ec1c5ef Compare September 5, 2024 10:56

BuonOmo added 2 commits September 5, 2024 12:58

maybe reset is the culprit

342fcb2

closer

4951a6b

BuonOmo force-pushed the bump-7-2 branch from 6de26e2 to 4951a6b Compare September 5, 2024 11:10

formigarafa mentioned this pull request Oct 15, 2024

Fix tests of #405 "Bump to 7.2.0" #412

Merged

tagliala reviewed Oct 16, 2024

View reviewed changes

test/cases/ddl_test.rb Outdated Show resolved Hide resolved

test/test_helper.rb Outdated Show resolved Hide resolved

formigarafa and others added 3 commits October 17, 2024 09:57

fix test teardown (#413)

4f6ebe6

typos

04dd527

Co-authored-by: Geremia Taglialatela <tagliala@users.noreply.github.com>

chore: add fundings

ac74f4e

BuonOmo marked this pull request as ready for review October 17, 2024 08:37

BuonOmo force-pushed the bump-7-2 branch from d1a195f to 96eb346 Compare October 17, 2024 08:39

cleanup

435027d

BuonOmo force-pushed the bump-7-2 branch from 96eb346 to 435027d Compare October 17, 2024 08:40

BuonOmo requested review from seuros and keithdoggett October 17, 2024 08:47

BuonOmo assigned keithdoggett Oct 17, 2024

seuros reviewed Oct 17, 2024

View reviewed changes

seuros self-requested a review October 17, 2024 13:20

keithdoggett approved these changes Oct 31, 2024

View reviewed changes

seuros approved these changes Nov 1, 2024

View reviewed changes

BuonOmo mentioned this pull request Nov 1, 2024

undefined method `migration_context' for an instance of ActiveRecord::ConnectionAdapters::PostGISAdapter (NoMethodError) #400

Closed

Bump version, update history and README

ecf8ef3

keithdoggett merged commit 1290093 into master Nov 4, 2024
16 checks passed

keithdoggett deleted the bump-7-2 branch November 4, 2024 20:30

mjy mentioned this pull request Nov 4, 2024

Gazetteer model SpeciesFileGroup/taxonworks#3979

Open

upendra-azad mentioned this pull request Nov 5, 2024

Error: uninitialized constant RGeo::ActiveRecord::SpatialToSql (NameError) after upgarding to newly released version 10 #416

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump to 7.2.0 #405

Bump to 7.2.0 #405

BuonOmo commented Sep 4, 2024

BuonOmo Sep 4, 2024

seuros Sep 4, 2024

BuonOmo Sep 4, 2024

seuros Sep 4, 2024

BuonOmo Sep 4, 2024

keithdoggett Sep 9, 2024

BuonOmo Sep 9, 2024

keithdoggett Sep 11, 2024

formigarafa Oct 14, 2024

seuros Sep 4, 2024

BuonOmo Sep 4, 2024

seuros Sep 5, 2024

BuonOmo Sep 5, 2024 •

edited

Loading

formigarafa commented Oct 13, 2024

formigarafa commented Oct 14, 2024 •

edited

Loading

tagliala left a comment

seuros left a comment

BuonOmo commented Oct 17, 2024

formigarafa commented Oct 25, 2024

BuonOmo commented Oct 25, 2024

keithdoggett left a comment

BuonOmo commented Nov 1, 2024

keithdoggett commented Nov 4, 2024

Bump to 7.2.0 #405

Bump to 7.2.0 #405

Conversation

BuonOmo commented Sep 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BuonOmo Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

formigarafa commented Oct 13, 2024

formigarafa commented Oct 14, 2024 • edited Loading

tagliala left a comment

Choose a reason for hiding this comment

seuros left a comment

Choose a reason for hiding this comment

BuonOmo commented Oct 17, 2024

formigarafa commented Oct 25, 2024

BuonOmo commented Oct 25, 2024

keithdoggett left a comment

Choose a reason for hiding this comment

BuonOmo commented Nov 1, 2024

keithdoggett commented Nov 4, 2024

BuonOmo Sep 5, 2024 •

edited

Loading

formigarafa commented Oct 14, 2024 •

edited

Loading