Closed alanking closed 2 months ago
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.
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.
Maybe I'm just repeating what @korydraughn has said, though ; )
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: https://github.com/irods/irods/issues/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?
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.
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.
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:
I think the first few are pretty obviously postgres-dependent, and were introduced here: https://github.com/irods/python-irodsclient/pull/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.