Fix: Some skipped tests

[syphax-bouazzouni]

Changes

Fix test_reentrant_queries test

I think (not totally sure) that the original goal of this test was to ensure that parallel reading and writing work. It creates two threads one that writes into the store and another one at the same time fetches data.

The test fails because it was expecting that the first thread (the writing one) was still alive after the end of the second one. But it wasn't the case the writing was too fast and ended first.

The solution was to make the first thread take more time (by sleeping for 1.5 seconds)

Fix test_embed_struct

It was already working, just skipped for no apparent reason

Update test_inverse_on_collection test

Still skipped, because it does not work or is not implemented even if it is an important feature.

Issue.in(john).all # works and return 5 issues
# but the inverse does not work 
User.find("John").include(:issues).first.issues # issues empty 

[ncbo-deployer]

I view the test_reentrant_queries as a bad test (that I will eventually drop) for several reasons. First of all, it is a test of threading and ruby has and will change its threading model. Most (non-virtual) machines have several cores (which helps I think). Secondly, since it is non-deterministic, it will run differently on different machines. Most developers now have fast machines where it may pass or fail somewhat consistently. Thirdly, it is looking for race conditions which are hard to test for. Say it passes 90% of the time in github but it always passes on your machine. What are you going to do? We had a Protege test like this where the reason for its intermittency was less obvious. Your other test I will have to look at.

…

-Timothy

________________________________ From: bioontology-admin ***@***.***> on behalf of Syphax bouazzouni ***@***.***> Sent: Saturday, December 17, 2022 8:31 AM To: ncbo/goo ***@***.***> Cc: Subscribed ***@***.***> Subject: [bioontology-admin] [ncbo/goo] Fix: Some skipped tests (PR #130) Changes Fix test_reentrant_queries test I think (not totally sure) that the original goal of this test was to ensure that parallel reading and writing work. It creates two threads one that writes into the store and another one at the same time fetches data. The test fails because it was expecting that the first thread (the writing one) was still alive after the end of the second one. But it wasn't the case the writing was too fast and ended first. The solution was to make the first thread take more time (by sleeping for 1.5 seconds) Fix test_embed_struct It was already working, just skipped for no apparent reason Update test_inverse_on_collection test Still skipped, because it does not work or is not implemented even if it is an important feature. Issue.in(john).all # works and return 5 issues # but the inverse does not work User.find("John").include(:issues).first.issues # issues empty

________________________________ You can view, comment on, or merge this pull request online at: #130 Commit Summary * 8e0e46d<8e0e46d> fix test_embed_struct in test_read_only.rb * 361940a<361940a> fix test_reentrant_queries by ensuring the write thread is still alive * a30af42<a30af42> update test_inverse_on_collection test File Changes (3 files<https://github.com/ncbo/goo/pull/130/files>) * M test/test_chunks_write.rb<https://github.com/ncbo/goo/pull/130/files#diff-b59f6f0f94e8dae726684f268c5c564d2282e622c59a787a467a298757d32c08> (5) * M test/test_collections.rb<https://github.com/ncbo/goo/pull/130/files#diff-84ae10b74f53836eb83ce090486276103f29679e3238ac65552c416193e08f4c> (18) * M test/test_read_only.rb<https://github.com/ncbo/goo/pull/130/files#diff-e691e1ed58301642a1b0343bc12b185a3887432065653db6acb8ab6b81f2e289> (10) Patch Links: * https://github.com/ncbo/goo/pull/130.patch * https://github.com/ncbo/goo/pull/130.diff — Reply to this email directly, view it on GitHub<#130>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAP2LDVM2J56SMMBBPT7TKTWNXTGXANCNFSM6AAAAAATCAYHR4>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[codecov-commenter]

Codecov Report

Merging #130 (a30af42) into master (077c674) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #130   +/-   ##
=======================================
  Coverage   85.27%   85.27%           
=======================================
  Files          20       20           
  Lines        2037     2037           
=======================================
  Hits         1737     1737           
  Misses        300      300           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[syphax-bouazzouni]

replaced with #150