Selet AI Tests for Vector Index. - #32
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
730fdcd to
4936204
Compare
| @pytest.fixture(scope="session") | ||
| def credential_connect_as(test_env): | ||
| def _connect_as(admin=False, **overrides): | ||
| select_ai.disconnect() |
There was a problem hiding this comment.
Please don't run select_ai.disconnect() in individual conftest.
We have seen this causes issues as it modifies the global connection state. We already have connect/disconnect in the root level conftest.py which is automatically run for every module.
https://github.com/oracle/python-select-ai/blob/main/tests/conftest.py#L201
We should not do any connection management in individual test suites.
There was a problem hiding this comment.
Removed additional connect/disconnect from subfolders.
| @pytest.fixture(scope="session") | ||
| def credential_async_connect_as(test_env): | ||
| async def _connect_as(admin=False, **overrides): | ||
| await select_ai.async_disconnect() |
There was a problem hiding this comment.
We already have async connect and disconnect here
https://github.com/oracle/python-select-ai/blob/main/tests/conftest.py#L208
No need to connect here
There was a problem hiding this comment.
Removed additional connect/disconnect from subfolders.
|
@Pushpendra-Garg please rebase changes from main. Thanks |
a480731 to
c06f8d4
Compare
Done. |
| pip install pytest anyio | ||
| pip install -e . | ||
|
|
||
| - name: Debug vector index secret wiring |
There was a problem hiding this comment.
Can we please remove this debug step ?
There was a problem hiding this comment.
Removed additional debug logs.
| @@ -0,0 +1,51 @@ | |||
| # ----------------------------------------------------------------------------- | |||
| # Copyright (c) 2025, Oracle and/or its affiliates. | |||
There was a problem hiding this comment.
For new files, Copyright header should use the current year - 2026
There was a problem hiding this comment.
Modified year to 2026 for all files.
| @@ -0,0 +1,415 @@ | |||
| # ----------------------------------------------------------------------------- | |||
| # Copyright (c) 2025, Oracle and/or its affiliates. | |||
| @@ -0,0 +1,356 @@ | |||
| # ----------------------------------------------------------------------------- | |||
| # Copyright (c) 2025, Oracle and/or its affiliates. | |||
There was a problem hiding this comment.
Please update the year in all files
| logger.info(f"SetUp for {method.__name__}") | ||
| self.objstore_cred = "OBJSTORE_CRED" | ||
| self.vecidx = select_ai.VectorIndex() | ||
| self.vector_index = list(self.vecidx.list(index_name_pattern=".*"))[0] |
There was a problem hiding this comment.
This will operate on whichever vector index happens to be returned first, not the index created in setup. In a shared schema or after earlier tests, this can validate/disable the wrong object. Fetch by exact name, for example index_name_pattern=f"^{self.index_name}$", and assert exactly one match.
There was a problem hiding this comment.
Modified test by fetching with exact name and assert.
|
|
||
| def test_2217(self): | ||
| """Test Credential creation for a local test user""" | ||
| test_username = "TEST_USER1" |
There was a problem hiding this comment.
We run tests for different Python versions in parallel. Creating TEST_USER1 is unsafe for parallel runs. Use UUID/suffix-based names derived from the test session, and always drop them in finally.
There was a problem hiding this comment.
Fixed by replacing TEST_USER1 with a UUID-suffixed local user and keeping user cleanup in finally.
|
|
||
| try: | ||
| for i in range(1, 6): | ||
| user = f"DB_USER{i}" |
There was a problem hiding this comment.
Don't use user names like DB_USER1, DB_USER2. In a shared CI infrastructure this can break parallel runs. Use UUID/suffix-based names derived from the test session, and always drop them in finally
There was a problem hiding this comment.
Fixed by using UUID-suffixed DB users instead of DB_USER1/DB_USER2 and dropping them in finally.
| """Test None as pattern input.""" | ||
| logger.info("Testing None as pattern input...") | ||
| indexes = self.vector_index.list(None) | ||
| assert len(list(indexes)) != len(self.indexes) |
There was a problem hiding this comment.
Yes, updated. The test now explicitly verifies that passing None to VectorIndex.list() does not fall back to the default match-all pattern ".*". Instead of only comparing counts, it checks that none of the managed test indexes are returned. The same clarification was applied to the async list test as well.
| """Test empty string as pattern input.""" | ||
| logger.info("Testing empty string as pattern input...") | ||
| indexes = self.vector_index.list("") | ||
| assert len(list(indexes)) != len(self.indexes) |
There was a problem hiding this comment.
The test now verifies that passing an empty string to VectorIndex.list("") does not fall back to the default match-all pattern ".*" by checking that none of the managed test indexes are returned.
813394d to
8065498
Compare
b5d7351 to
970d555
Compare
56d751d to
ea6b925
Compare
ea6b925 to
8614d2f
Compare
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: