search

rgeo / activerecord-postgis-adapter

ActiveRecord connection adapter for PostGIS, based on postgresql and rgeo
Other
885 stars 242 forks source link

Some ActiveRecord Tests Failing #378

Open keithdoggett opened 1 year ago

keithdoggett commented 1 year ago

Running bundle exec rake test:activerecord yields ~50 test failures, currently. Most of these need to be excluded and re-written in our own test suite. Once I have a standard process for excluding ActiveRecord tests I'll update this issue.

BuonOmo commented 2 months ago

@keithdoggett I'm back on running this test suite, and excluding tests with minitest-exclude. But as of now something quite weird happens: in CI (not locally) tests are super slow when use_transactional_tests = false, and they seem to get exponentially slower in a given class. Example from one of my latest CI tests:

PostgresqlHstoreTest#test_array_cycle = 0.07 s = .
PostgresqlHstoreTest#test_array_strings_with_array_delimiters = 0.08 s = .
PostgresqlHstoreTest#test_array_strings_with_commas = 0.09 s = .
PostgresqlHstoreTest#test_array_strings_with_null_strings = 0.15 s = .
PostgresqlHstoreTest#test_array_strings_with_quotes = 0.42 s = .
PostgresqlHstoreTest#test_arrow = 1.40 s = .
PostgresqlHstoreTest#test_backslash = 5.20 s = .
PostgresqlHstoreTest#test_cast_value_on_write = 20.81 s = .
PostgresqlHstoreTest#test_change_table_supports_hstore = 82.67 s = .
PostgresqlHstoreTest#test_changes_in_place = 333.01 s = .
PostgresqlHstoreTest#test_changes_with_store_accessors = 1365.47 s = .
PostgresqlHstoreTest#test_clone_hstore_with_serialized_attributes = 5903.55 s = .
... (still running!)

Any hint on this? Seems fun to debug anyway, the only issue is that it is CI specific.

Culprit found: https://github.com/ged/ruby-pg/issues/586

BuonOmo commented 2 months ago

I've added the test suite and excluded tests failing with the TRIAGE_MESSAGE constant (rg TRIAGE MESSAGE yields them all). Anyone can take some of these once #405 is merged, they are now truly a good first issue.

Note that some skipped tests should not be skipped as well. We should mention them somewhere in our backlog..

keithdoggett commented 2 months ago

@BuonOmo agree about creating a backlog for those tests. Could probably make an issue for each one with some kind of new label.

For the slow tests is there any action we can take? Seems like an upstream issue. Thanks for digging into that.

BuonOmo commented 2 months ago

Could probably make an issue for each one with some kind of new label.

I wouldn't as it would clutter the backlog a lot.

For the slow tests is there any action we can take? Seems like an upstream issue.

Since PG is no official dependency of this lib, there's no action of upgrading pg for users. Just in tests (which is done in the bump 7.2 pr)

formigarafa commented 1 month ago

I've made a gist with a list of them it may help: https://gist.github.com/formigarafa/5e5b5687fe5d09535affd2afc7df3c68