-
Notifications
You must be signed in to change notification settings - Fork 19
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
[PM-11873] Add Database Testing Instructions #480
Open
justindbaur
wants to merge
8
commits into
main
Choose a base branch
from
database-testing
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.
+93
โ0
Open
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b4361c5
Initial Database Testing Document
justindbaur dc0dcd8
Merge remote-tracking branch 'origin/main' into database-testing
justindbaur 1358860
Fix JSON
justindbaur 3bc8da5
Merge branch 'main' into database-testing
withinfocus 1725368
Apply suggestions from code review
justindbaur 6c4c99e
Apply suggestions from code review
justindbaur 7cd0f8a
Prettier
justindbaur 5b13c27
Remove `4`
justindbaur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
# Database Integration Testing | ||
|
||
Since Bitwarden has multiple database options (4), testing them all automatically is incredibly | ||
important so that we don't have to have a full QA process for each database type. This is where the | ||
`Infrastructure.IntegrationTest` project comes in, it allows setting up tests similarly to how the | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
databases are consumed in the application. Through their common interface, generally an | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`I*Repository`. These tests are automatically ran on all supported databases in our testing | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pipeline. | ||
|
||
## Creating a new test | ||
|
||
To create a new database test, just add the `[DatabaseTheory]` and `[DatabaseData]` attributes to | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
test. Now, in the parameters of the test you are able to "inject" any repository layer services | ||
directly into the test. The test will run for every database that is | ||
[configured in the current environment](#configure-the-tests). Since you inject the interface of the | ||
service, some runs of the test will use the Dapper based repository implementation targeting | ||
Microsoft SQL Server and others will use the Entity Framework Core based implementations, which we | ||
use for MySql, Postgres, and SQLite. | ||
|
||
The goal of database tests is to test the business logic that is encapsulated in a given method. If | ||
the stored procedure in SQL Server then calls another procedure to update the | ||
`User.AccountRevisionDate` then the same EF implementation should do that as well. Then by running | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
the test against all variants, we are ensuring all the variants are feature equal. Locally, you may | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
want to only the SQL Server tests along with one EF implementation, SQLite is often the easiest. | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This may work well for a very long time and save you some time overall but there are differences | ||
between the EF database providers such that you will one day get errors in the CI pipeline. | ||
|
||
## Configure the databases | ||
|
||
The databases are expected to have the latest migrations applied to them. | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Configure the tests | ||
|
||
The tests are configured through | ||
[.NET Configuration](https://learn.microsoft.com/en-us/dotnet/core/extensions/configuration). In the | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
order they are applied, user secrets, environment variables prefixed with `BW_TEST_`, and command | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
line args. | ||
|
||
```C# | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public class Root | ||
{ | ||
public Database[] Databases { get; set; } | ||
} | ||
|
||
public class Database | ||
{ | ||
public SupportedDatabaseProviders Type { get; set; } | ||
public string ConnectionString { get; set; } = default!; | ||
public bool UseEf { get; set; } | ||
public bool Enabled { get; set; } = true; | ||
} | ||
``` | ||
|
||
The `Type` property is an enum with the supported values being `SqlServer`, `MySql`, `Postgres`, or | ||
`Sqlite`. The `UseEf` property is only utilized it the `Type` is set to `SqlServer`, by default | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`SqlServer` will be configured with the Dapper repositories but by setting `UseEf` to `true`, it | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
will be configured with the Entity Framework Core repositories. `Enabled` allows you to easily | ||
disable one database but not delete the entry, it can be helpful if you are encountering a problem | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
with just a single database type and want to run the tests just for it instead of for all of them. | ||
|
||
### Locally | ||
|
||
To set the tests up locally you may want to add the configuration to your `dev/secrets.json` file. | ||
An example entry you might add is: | ||
|
||
```json | ||
{ | ||
...other config... | ||
"databases": [ | ||
{ | ||
"type": "SqlServer", | ||
"connectionString": "myConnectionString" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
You can also configure the tests just like the pipeline, read more below. | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Pipeline | ||
|
||
The database tests have been pre-configured to run on all supported databases in the | ||
[`test-database.yml`](https://github.com/bitwarden/server/blob/main/.github/workflows/test-database.yml) | ||
([permalink](https://github.com/bitwarden/server/blob/f7bc5dfb2ea31ca7b4c36238295cdcc4008ad958/.github/workflows/test-database.yml)) | ||
file. | ||
|
||
The pipeline uses environment variables, an example entry you might add is: | ||
justindbaur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```bash | ||
BW_TEST_DATABASES__0__TYPE: SqlServer | ||
BW_TEST_DATABASES__0__CONNECTIONSTRING: myConnectionString | ||
``` |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐จ I'd leave "4" out of this, as it could potentially change and this would be an easy miss.