test: enhance integration test reliability and resource cleanup #2207
Conversation
Summary of ChangesHello @rahulpinto19, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability of integration tests across various database systems by implementing robust pre-creation cleanup mechanisms and unique resource naming strategies. These changes ensure a clean testing environment for each run, preventing interference from previous test failures and avoiding naming conflicts. Additionally, the PR includes an update to the developer documentation regarding link checking practices. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the reliability of integration tests by introducing pre-creation cleanup and unique naming for test resources. These are excellent changes for test stability. My review includes suggestions to further improve the code by handling errors in cleanup steps, removing commented-out code, and addressing minor formatting and typos.
rahulpinto19
force-pushed
the
integration-test
branch
2 times, most recently
from
c5e9765 to
b7c2b1a
Compare
rahulpinto19
changed the title
|
Please clean up the commit history |
|
Please reformat the PR title accordingly |
rahulpinto19
force-pushed
the
integration-test
branch
from
ddd75e6 to
a02ca45
Compare
rahulpinto19
force-pushed
the
integration-test
branch
2 times, most recently
from
99f728f to
f73d466
Compare
## Description The list_database_stats test fails intermittently when run in parallel on shared instances. Specifically, the "filter by tablespace" and "sort by size" test cases fail because they encounter unexpected databases in the pg_default tablespace created by concurrent test runs. This PR narrows the scope of these test cases by filtering for specific database names. This ensures assertions remain isolated to the current test run regardless of other databases present in the shared environment. ``` go test -tags=integration tests/postgres/postgres_integration_test.go ok command-line-arguments 14.455s ``` > Should include a concise description of the changes (bug or feature), it's > impact, along with a summary of the solution ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #<1738>
Added safety drop for existing table before creation.
rahulpinto19
changed the title
| } | ||
|
|
||
| // Drop the target collection used in aggregate tests | ||
| if err := database.Collection("target_collection").Drop(ctx); err != nil { |
There was a problem hiding this comment.
What if a collection is created by another test execution and they are still using it? Dropping the collection here will cause the other test using it to fail.
| // Safety drop before creation | ||
| _, err = pool.ExecContext(ctx, fmt.Sprintf("DROP TABLE IF EXISTS %s;", tableName)) | ||
| if err != nil { | ||
| t.Fatalf("Warning: failed to drop table %s before creation:%v ", tableName, err) |
There was a problem hiding this comment.
Same here. We should not cause failure in other tests.
4 participants
This PR improves the reliability of the collection creation process by introducing two key changes:
Pre-creation Cleanup: It now clears existing collections before a new one is initialized to ensure a clean state.
Collision Prevention: It implements unique table naming logic to avoid naming conflicts and resource collisions during the creation process.