Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failures with mysql and mariadb #597

Closed
alanking opened this issue Jul 30, 2024 · 6 comments
Closed

Test failures with mysql and mariadb #597

alanking opened this issue Jul 30, 2024 · 6 comments
Assignees
Labels
Milestone

Comments

@alanking
Copy link
Contributor

alanking commented Jul 30, 2024

When running tests against the tip of main on an iRODS 4.3.3-ish server I saw the following tests fail when using a mysql 8.4 or mariadb 11.4 server for hosting the iRODS catalog:

irods.test.genquery2_test.TestGenQuery2.test_select
irods.test.genquery2_test.TestGenQuery2.test_select_and
irods.test.genquery2_test.TestGenQuery2.test_select_or
irods.test.genquery2_test.TestGenQuery2.test_select_with_explicit_zone
irods.test.query_test.TestQuery.test_files_query_case_sensitive

Here are the details:

======================================================================     
FAIL [0.184s]: test_select (irods.test.genquery2_test.TestGenQuery2)     
----------------------------------------------------------------------     
Traceback (most recent call last):     
  File "/var/lib/irods/python-irodsclient/irods/test/genquery2_test.py", line 38, in test_select     
    self.assertEqual(query_sql, "select distinct t0.coll_name from R_COLL_MAIN t0 inner join R_OBJT_ACCESS pcoa on t0.coll_id = pcoa.object_id inner join R_TOKN_MAIN pct on pcoa.access_type_id = pct.token_id inner join R_USER_MAIN pcu on pcoa.user_id = pcu.user_id where t
0.coll_name = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only")
AssertionError: 'sele[246 chars] = ? and pcoa.access_type_id >= 1000 limit 256' != 'sele[246 chars] = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only'
Diff is 1225 characters long. Set self.maxDiff to None to see it.                                                                       
     
======================================================================
FAIL [0.176s]: test_select_and (irods.test.genquery2_test.TestGenQuery2)
----------------------------------------------------------------------     
Traceback (most recent call last):                                           
  File "/var/lib/irods/python-irodsclient/irods/test/genquery2_test.py", line 86, in test_select_and     
    self.assertEqual(query_sql, "select distinct t0.coll_name from R_COLL_MAIN t0 inner join R_OBJT_ACCESS pcoa on t0.coll_id = pcoa.object_id inner join R_TOKN_MAIN pct on pcoa.access_type_id = pct.token_id inner join R_USER_MAIN pcu on pcoa.user_id = pcu.user_id where t
0.coll_name like ? and t0.coll_name like ? and pcoa.access_type_id >= 1000 fetch first 256 rows only")                                       
AssertionError: 'sele[273 chars]ke ? and pcoa.access_type_id >= 1000 limit 256' != 'sele[273 chars]ke ? and pcoa.access_type_id >= 1000 fetch first 256 rows only'                                                                                                              Diff is 1333 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL [0.270s]: test_select_or (irods.test.genquery2_test.TestGenQuery2)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/irods/python-irodsclient/irods/test/genquery2_test.py", line 75, in test_select_or
    self.assertEqual(query_sql, "select distinct t0.coll_name from R_COLL_MAIN t0 inner join R_OBJT_ACCESS pcoa on t0.coll_id = pcoa.object_id inner join R_TOKN_MAIN pct on pcoa.access_type_id = pct.token_id inner join R_USER_MAIN pcu on pcoa.user_id = pcu.user_id where t
0.coll_name = ? or t0.coll_name = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only")
AssertionError: 'sele[266 chars] = ? and pcoa.access_type_id >= 1000 limit 256' != 'sele[266 chars] = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only'
Diff is 1305 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL [0.308s]: test_select_with_explicit_zone (irods.test.genquery2_test.TestGenQuery2)
----------------------------------------------------------------------
Traceback (most recent call last): 
  File "/var/lib/irods/python-irodsclient/irods/test/genquery2_test.py", line 49, in test_select_with_explicit_zone
    self.assertEqual(query_sql, "select distinct t0.coll_name from R_COLL_MAIN t0 inner join R_OBJT_ACCESS pcoa on t0.coll_id = pcoa.object_id inner join R_TOKN_MAIN pct on pcoa.access_type_id = pct.token_id inner join R_USER_MAIN pcu on pcoa.user_id = pcu.user_id where t
0.coll_name = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only")
AssertionError: 'sele[246 chars] = ? and pcoa.access_type_id >= 1000 limit 256' != 'sele[246 chars] = ? and pcoa.access_type_id >= 1000 fetch first 256 rows only'
Diff is 1225 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL [0.770s]: test_files_query_case_sensitive (irods.test.query_test.TestQuery)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/irods/python-irodsclient/irods/test/query_test.py", line 188, in test_files_query_case_sensitive
    self.assertEqual(len(result13), 1)
AssertionError: 2 != 1                                              

I think the first few are pretty obviously postgres-dependent, and were introduced here: #555

I'm not sure about that last one. Maybe it has to do with case sensitive / insensitive search and defaults, or something.

Anyway, we should investigate this.

@korydraughn
Copy link
Contributor

The tests shouldn't assert equality of the SQL generated by GenQuery2 since it can change over time.

Those assertions need to be removed or changed to assert specific character sequences appear in the result.

@d-w-moore
Copy link
Collaborator

I think one shouldn't have to assert those anyway. The only needed assertions should be that the results of a query match the expectations of the test, based on the conditions that were set up.

@d-w-moore
Copy link
Collaborator

Maybe I'm just repeating what @korydraughn has said, though ; )

@alanking
Copy link
Contributor Author

alanking commented Aug 1, 2024

That last test is also a subtle assumption about the database being postgres. This test failure spawned this issue, so I put the details over here: irods/irods#7930

We just need to tweak the test to change the assertion. However, I'm not sure what we want to assert here:

Traceback (most recent call last):
  File "/var/lib/irods/python-irodsclient/irods/test/query_test.py", line 188, in test_files_query_case_sensitive
    self.assertEqual(len(result13), 1)
AssertionError: 2 != 1

For postgres, we want to assert that only 1 result came back because there is only 1 data object with the name for which we are searching if the query is case sensitive (as it should be in this test). For mysql, the between clause will effectively return a case-insensitive resultset and this is also correct for the mysql implementation of the between clause. If we assert "at least 1" result, it feels like the test is moot because running a case-insensitive search would pass that assertion, too.

I'm thinking about splitting this one out into a separate issue and just commenting out this part of the test until we can decide on what to do with it. This does not seem like a blocker for 2.1.0 to me. Thoughts?

@korydraughn
Copy link
Contributor

As long as the PRC can demonstrate hitting the GenQuery2 API endpoint successfully, that's all that matters.

Making assertions about the SQL returned by the GenQuery2 API isn't important for the PRC. If we must cover that case, then a few assertions like the following are good enough.

assertTrue(SQL.startswith('select '))
assertTrue(' R_COLL_NAME ' in SQL)

Given that GenQuery2 is experimental, it shouldn't block a release.

alanking added a commit to alanking/python-irodsclient that referenced this issue Aug 1, 2024
Many of the tests in genquery2_tests.py include assertions on the
specific SQL strings generated by GenQuery2. These assertions assume
a Postgres database, causing the tests to fail on non-Postgres databases
which are nonetheless supported by the iRODS server. These assertions
have been replaced.

This changes the Postgres-specific assertions in many of the genquery2_test
tests to just assert that a table name is returned. We cannot assert the
specific contents without reaching out to the server to detect the version
and flavor of the database. Even then, the specific results may vary over
time. Testing the generated SQL is more the responsibility of the GenQuery2
parser and API in the server anyway, so this test should just be asserting
that a result that sort of looks like what we want is being returned by the
library.
alanking added a commit that referenced this issue Aug 1, 2024
Many of the tests in genquery2_tests.py include assertions on the
specific SQL strings generated by GenQuery2. These assertions assume
a Postgres database, causing the tests to fail on non-Postgres databases
which are nonetheless supported by the iRODS server. These assertions
have been replaced.

This changes the Postgres-specific assertions in many of the genquery2_test
tests to just assert that a table name is returned. We cannot assert the
specific contents without reaching out to the server to detect the version
and flavor of the database. Even then, the specific results may vary over
time. Testing the generated SQL is more the responsibility of the GenQuery2
parser and API in the server anyway, so this test should just be asserting
that a result that sort of looks like what we want is being returned by the
library.
@alanking alanking self-assigned this Aug 1, 2024
@alanking
Copy link
Contributor Author

alanking commented Aug 1, 2024

The genquery2 tests have been fixed and are now passing for all the database flavors we support. The irods.test.query_test.TestQuery.test_files_query_case_sensitive test will be handled separately in #600 because it is a special case. Closing this one.

@alanking alanking closed this as completed Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants