tapdb+mssmt: run tests against registered db backends (redux)#2117
tapdb+mssmt: run tests against registered db backends (redux)#2117jtobin wants to merge 2 commits into
Conversation
Summary of ChangesHello, 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 enhances the testing infrastructure for the MSSMT component by enabling the execution of unit tests against real SQL database backends. By registering database drivers, the test suite can now exercise both SQLite and Postgres implementations, which were previously bypassed in favor of an in-memory store. This change is intended to improve test coverage and identify potential issues related to database interactions. 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. 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 refactors the tree store testing infrastructure by implementing a driver registration pattern for Postgres and SQLite3, allowing genTestStores to dynamically instantiate stores. It also removes the unused NewDbHandleFromPath function and updates relevant documentation. Feedback was provided to minimize the number of lines used for error messages in accordance with the style guide and to add bounds checks for variadic arguments in the driver factory functions to prevent potential runtime panics.
jtobin
force-pushed
the
mssmt-unit-db-backends
branch
from
918138c to
657fd71
Compare
Register the active SQL backend as a TreeStoreDriver in init() functions within the build-tag gated test_sqlite.go and test_postgres.go files. The driver's New func accepts a testing.TB through its variadic args, giving both backends proper cleanup via t.Cleanup. Remove the unused NewDbHandleFromPath from sqlutils_test.go.
Remove the sqlite3 special-case in genTestStores. The function now emits constructors for all registered tree store drivers, passing the test's testing.T through to driver.New so backends can use it for setup and cleanup.
|
This now depends on #2119 in order to pass CI. |
2 participants
Replaces #770.
The mssmt unit tests use a driver registration pattern to discover tree store backends. Previously, no backend was ever registered, so only the default in-memory store was used.
These two commits wire up the SQL backends so the unit tests actually exercise them. Each build-tag-gated file now registers a tree store driver during initialization, and we construct a store for every registered driver.
This immediately surfaced #2116: the insert queries for mssmt_nodes aren't idempotent, so tests that produce duplicate node hashes fail against SQL backends (e.g. sqlite3, postgres). Those tests always passed before because they were only running against the in-memory store.