Arvados

Branch 10223-cr-set-output-name: 6599088
Test run: https://ci.curoverse.com/job/developer-run-tests/126/

@ 6599088b

• Can you please break the new test into two tests: (1) using some output name that is acceptable, (2) an output_name such as "foo_file" that is already existing on a collection fixture owned by the 'active' user? Alternatively, you can just use (2) since it will cover case (1) also.

• Does the updated test eliminate the need for this? ActiveRecord::Base.connection.execute 'ROLLBACK'

• I noticed that we unfortunately do not have a test such as "full text search indexes are up to date" similar to "search index exists on models that go into projects" which is resulting in the fts index being out of sync with the code updates. Please add a subtask to update the fts index as well (and add that new test). Thanks.

Radhika Chippada wrote:

• Can you please break the new test into two tests: (1) using some output name that is acceptable, (2) an output_name such as "foo_file" that is already existing on a collection fixture owned by the 'active' user? Alternatively, you can just use (2) since it will cover case (1) also.

Done.

• Does the updated test eliminate the need for this? ActiveRecord::Base.connection.execute 'ROLLBACK'

No, because every unit test in run inside a transaction. I added a comment explaining why that execution is there.

• I noticed that we unfortunately do not have a test such as "full text search indexes are up to date" similar to "search index exists on models that go into projects" which is resulting in the fts index being out of sync with the code updates. Please add a subtask to update the fts index as well (and add that new test). Thanks.

Also, done.

Updates: c9199d2
Test run: https://ci.curoverse.com/job/developer-run-tests/130/

Lucas: thank you very much for adding the missing unit test for full text search indexes. LGTM