Some test and test infrastructure fixes and improvements #601
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Let's go with option 2 for #597. That is, assert that the SQL contains an expected table name. At the very least, it proves that the API is returning something. |
korydraughn
reviewed
Aug 1, 2024
There was a problem hiding this comment.
Changes look good to me.
Will approve after one of the #597 commits is removed.
alanking
commented
Aug 1, 2024
|
If there are no further comments, I'll squash this down to two commits which would include the table name assertions (option 2). |
|
Oops, I mean 3 commits. 1 for the login_auth_test rename, 1 for the genquery2_test, and 1 for commenting out the BETWEEN tests. |
|
If you're happy with it, squash to taste. |
|
Squashed |
The runner.py script which is used to run the full test suite looks for files with the pattern "*_test.py". login_auth_test.py requires a special environment and so should not be run with the regular suite of tests. It must be run independently and manually, so the login_auth_test.py file has been renamed to login_auth_test_must_run_manually.py in order to allow it to continue to exist and be run, but not along with the rest of the suite.
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.
The BETWEEN clause behaves differently for mysql (and mariadb, I assume) than postgres, and the test_files_query_case_sensitive test assumes that we are using postgres. In order to avoid this, the BETWEEN sub-tests have been commented out until such a time as we can make meaningful assertions about the case-sensitive query with a BETWEEN clause based on the flavor of database being used by the connected iRODS server.
|
#'d. Mergin. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #566 (I think)
Addresses #597 (via two alternatives)
The #566 solution seemed the most expedient, and we will need to revisit that one as part of the effort for #502. Willing to work on the name.
The #597 commits are alternatives, so we need to pick one. The options are:
I do not have a preference for 1 or 2 over the other. I kind of feel like the assertion is only there to assert that something is returned. The specifics of the contents seems more like the job of the GenQuery2 parser and/or API as far as testing goes, IMO.
Test suite as determined by
irods/test/runner.pynow passes running against iRODS 4.3.2 with postgres and mysql.