-
Notifications
You must be signed in to change notification settings - Fork 112
Without yaml #3623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ScottDugas
wants to merge
33
commits into
FoundationDB:main
Choose a base branch
from
ScottDugas:without-yaml
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Without yaml #3623
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
This changes the way cluster files are specified for the tests, introducing a new yaml file. This makes it more amenable to having more than one cluster file. The fdb-environment.properties is still supported so developers don't immediately have to update their configuration.
This took a lot of trial and error in CI, and adding and removing a lot of logging and investigative code. The key thing that tripped me up was the fact that you have to do `configure new ssd` if it is a new cluster, which we were not doing previously, because the installation created the cluster.
This might be a bit of overkill, if they aren't using FDBDatabaseExtension, but it probably avoids confusion later on.
it was trying to connect to the default, which doesn't exist
…stCopyInSameDatabase
The reason for the issue is that it was reassigning `fdb = dbExtension.getDatabase()`, but it loaded data into `fdb` in a `beforeEach`, so if it picked different clusters it wouldn't have any data, and execute wouldn't fail. In debugging I also removed teh second `openRecordStore`, because the state should respond to calling `markIndexWriteOnly`, you shouldn't have to re-read, and we actually say you should never open the same store with different objects in the same transaction. I also isolated the exception check, and switched to use assertJ for the assertion, because it ends up being quite a bit cleaner than with junit's assertions.
I tried putting it in testFixtures, but that doesn't play well with other sub-projects already depending on the tests configuration
Doing so requires moving everything to testFixtures, so best to do in a separate PR
… it uses in setup
I think in the long term we don't want the driver to work like this, but I'm not sure how we want to have it work, and creating an api for that seems like more work than it is worth at this time.
…tions The external yaml connections do not support some operations that aren't really needed in mixed-mode, so that needs the cluster file from the connection.
I think people could be confused by getting different clusters each call to `getDatabase`, so this persists that. If you want to test against multiple clusters, use the indexed methods.
a784835
to
621944b
Compare
This validates that if you don't have fdb-environment.yaml the tests still pass (except the one with an assumption check)
d3d0376
to
c0590d3
Compare
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.
No description provided.