Arvados

Updates at 0e9aa62c7 - branch 14966-api-test-hang-fix
Test run: https://ci.curoverse.com/job/developer-run-tests/1123/

• Facts
  • Minitest has a feature that’s active by default called ’transactional fixtures’ that encloses every test in a transaction so it’s rolled back before running the next.
  • Nested transactions share the same database connection (https://apidock.com/rails/ActiveRecord/ConnectionAdapters/DatabaseStatements/transaction)
  • The permissions graph update is run inside a transaction
• Hypothesis
  • When a permission-changing operation is made within a test, we’re getting nested transactions that share the same database connection. This is usually fine because both run within the same thread, but for this special case that we expect the inner transaction to run concurrently in its own thread, it doesn’t happen because the connection is already occupied and sits waiting to be released before continuing.
• Fix
  • Although I already tried to manually ask for a new connection using ActiveRecord::Base.connection_pool.with_connection do ... end on refresh_permission_view(), it seems there’s no obvious way to ask for a new connection so instead the proposed fix would be to move this special test on its own test case class, disabling transactional fixtures.
  • The “cons” that this has is that the changes made on this test would leak to other tests, so we have to make sure to clean up before finishing.

Lucas Di Pentima wrote:

Updates at 0e9aa62c7 - branch 14966-api-test-hang-fix
Test run: https://ci.curoverse.com/job/developer-run-tests/1123/

• Facts
  • Minitest has a feature that’s active by default called ’transactional fixtures’ that encloses every test in a transaction so it’s rolled back before running the next.
  • Nested transactions share the same database connection (https://apidock.com/rails/ActiveRecord/ConnectionAdapters/DatabaseStatements/transaction)
  • The permissions graph update is run inside a transaction
• Hypothesis
  • When a permission-changing operation is made within a test, we’re getting nested transactions that share the same database connection. This is usually fine because both run within the same thread, but for this special case that we expect the inner transaction to run concurrently in its own thread, it doesn’t happen because the connection is already occupied and sits waiting to be released before continuing.
• Fix
  • Although I already tried to manually ask for a new connection using ActiveRecord::Base.connection_pool.with_connection do ... end on refresh_permission_view(), it seems there’s no obvious way to ask for a new connection so instead the proposed fix would be to move this special test on its own test case class, disabling transactional fixtures.
  • The “cons” that this has is that the changes made on this test would leak to other tests, so we have to make sure to clean up before finishing.

I'm not sure I totally understand the explanation, but agree that transactions / connection pooling could be causing trouble. Since it is an integration test it seems reasonable to run the test without the an outer transaction. Couple notes:

The test should be wrapped in a begin ... ensure .. end.

To reset the database, suggest using the database reset endpoint:

    post '/database/reset', {}, admin_auth
    assert_response :success

typo "accesible" -> "accessible"

Updates at 0145b3654
Test run: https://ci.curoverse.com/job/developer-run-tests/1126/

Instead of resetting the database inside an ensure block, do it at test case's teardown.