multi: various test preparations for different graph store impl #9800
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
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
from
b16ab27 to
34b1d9b
Compare
84 tasks
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
2 times, most recently
from
d1e39a7 to
254fdb4
Compare
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
2 times, most recently
from
2b2c6e1 to
3bb811f
Compare
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
2 times, most recently
from
98b79ab to
7121f04
Compare
|
(note: the |
|
fixing the merge conflicts now (due to rebase of target branch on #9804) |
To clearly separate the mission control DB store (which for now will remain kvdb only) from the graph store, we rename the `graphBackend` variable to `mcBackend` in the `testGraphInstance` struct.
Currently none of the calls to MakeTestGraph make use of the KVStoreOptionModifier options but later on we will want to make use of the `WithUseGraphCache` ChannelGraphOption and so we take this opportunity to switch out the functional parameters that the helper function takes.
In this commit, we unify how all unit tests that make use of the graph create their test ChannelGraph instance. This will make it easier to ensure that once we plug in different graph DB implementations, that all unit tests are run against all variants of the graph DB. With this commit, `NewChannelGraph` is mainly only called via `MakeTestGraph` for all tests _except_ for `TestGraphLoading` which needs to be able to reload a ChannelGraph with the same backend. This will be addressed in a follow-up commit once more helpers are defined. Note that in all previous packages where we created a test graph using `kvdb.GetBoltBackend`, we now need to add a `TestMain` function with a call to `kvdb.RunTest` since the `MakeTestGraph` helper uses `GetTestBackend` instead of `kvdb.GetBoltBackend` which requires an embedded postgres instance to be running.
In this commit, we add a `test_kvdb.go` file with a single definition of the `NewTestDB` function. A new version of `MakeTestGraph` (called `MakeTestGraphNew` is added which makes use of this `NewTestDB` function to create the backing `V1Store` passed to the `ChannelGraph` for tests. Later on, we will add new implementations of this method backed by sqlite and postgres. When those are added, then build flags will control which version of `NewTestDB` is called. With this change, the only test call-site of `NewKVStore` is the new `test_kvdb.go` file.
Update the test methods to take a `testing.TB` param instead of a `*testing.T` so that the test functions can be used for all the graph's tests including benchmark tests. We also add a temporary replace so that we can make use of these changes and also since we will soon be adding graph related schemas and queries in this submodule that we'll want to have access to.
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
from
7121f04 to
12dd4e3
Compare
In this commit, we implement the postgres and sqlite versions of the NewTestDB function. We add the various build flags so that only one of the three versions of this function can be active at a time. We also introduce the SQLStore struct which is the SQL implementation of the V1Store interface. NOTE: it currently temporarily embeds the KVStore struct so that we can implement the V1Store interface incrementally. For any method not implemented, things will fall back to the KVStore. This is ONLY the case for the time being while this struct is purely used in unit tests only. Once all the methods have been implemented, the KVStore field will be removed from the SQLStore struct.
Add `migrations_dev.go` and `migrations_prod.go` files which each define a `migrationAdditions` slice to be appended to the `migrationConfig` slice. The `migrations_dev.go` file is only built if either the `test_db_postgres` or `test_db_sqlite` build flags are used. This slice will be used to add any migrations that are still under development that we want access to via unit tests and itests but do not want to expose to our release build.
ellemouton
force-pushed
the
graphSQL5-unified-test-graph
branch
from
12dd4e3 to
8101f95
Compare
ellemouton
merged commit b857ae5
into
lightningnetwork:elle-graph
35 of 36 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In this PR we start prepping various unit tests so that we can easily plug in a different
V1Storeimplementationsimply by later adding new test build flags that will result in different versions of
NewTestDBbeing compiled.We prepare for this here by ensuring that all test
ChannelGraphs are created via a singleMakeTestGraphhelperwhich uses this new
NewTestDBhelper.We also add the skeleton for the new
SQLStorewhich we will step by step use to implement the sql versionof the
V1Store- only the framework is added in this PR. Reviewers can take a look at this PoC which shows how this framework will be used to implement theSQLStore in a piece-meal manner.
Part of #9795