chore: tests are timing out #9264
Merged
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
ivarconr
approved these changes
Feb 9, 2025
gastonfournier
added a commit
that referenced
this pull request
Feb 11, 2025
## About the changes Based on the first hypothesis from #9264, I decided to find an alternative way of initializing the DB, mainly trying to run migrations only once and removing that from the actual test run. I found in [Postgres template databases](https://www.postgresql.org/docs/current/manage-ag-templatedbs.html) an interesting option in combination with jest global initializer. ### Changes on how we use DBs for testing Previously, we were relying on a single DB with multiple schemas to isolate tests, but each schema was empty and required migrations or custom DB initialization scripts. With this method, we don't need to use different schema names (apparently there's no templating for schemas), and we can use new databases. We can also eliminate custom initialization code. ### Legacy tests This method also highlighted some wrong assumptions in existing tests. One example is the existence of `default` environment, that because of being deprecated is no longer available, but because tests are creating the expected db state manually, they were not updated to match the existing db state. To keep tests running green, I've added a configuration to use the `legacy` test setup (24 tests). By migrating these, we'll speed up tests, but the code of these tests has to be modified, so I leave this for another PR. ## Downsides 1. The template db initialization happens at the beginning of any test, so local development may suffer from slower unit tests. As a workaround we could define an environment variable to disable the db migration 2. Proliferation of test dbs. In ephemeral environments, this is not a problem, but for local development we should clean up from time to time. There's the possibility of cleaning up test dbs using the db name as a pattern: https://github.com/Unleash/unleash/blob/2ed2e1c27418b92e815d06c351504005cf083fd0/scripts/jest-setup.ts#L13-L18 but I didn't want to add this code yet. Opinions? ## Benefits 1. It allows us migrate only once and still get the benefits of having a well known state for tests. 3. It removes some of the custom setup for tests (which in some cases ends up testing something not realistic) 4. It removes the need of testing migrations: https://github.com/Unleash/unleash/blob/main/src/test/e2e/migrator.e2e.test.ts as migrations are run at the start 5. Forces us to keep old tests up to date when we modify our database
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.
About the changes
I figured that many of our tests are timing out and we can solve this by increasing test timeout.
This is a signal that either our tests became slower or that our query performance got worse. On the first point it might be due to having more migrations than before and requiring more time. The second point is harder to validate but something to keep an eye on