Skip to content

Commit

Permalink
chore: Create test db from template (#9265)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
gastonfournier authored Feb 11, 2025
1 parent a8ea174 commit 5e9698f
Show file tree
Hide file tree
Showing 40 changed files with 253 additions and 306 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@
"test:report": "NODE_ENV=test PORT=4243 jest --reporters=\"default\" --reporters=\"jest-junit\"",
"test:docker:cleanup": "docker rm -f unleash-postgres",
"test:watch": "yarn test --watch",
"test:coverage": "NODE_ENV=test PORT=4243 jest --coverage --testLocationInResults --outputFile=\"coverage/report.json\" --forceExit --testTimeout=10000",
"test:coverage:jest": "NODE_ENV=test PORT=4243 jest --silent --ci --json --coverage --testLocationInResults --outputFile=\"report.json\" --forceExit --testTimeout=10000",
"test:updateSnapshot": "NODE_ENV=test PORT=4243 jest --updateSnapshot --testTimeout=10000",
"test:coverage": "NODE_ENV=test PORT=4243 jest --coverage --testLocationInResults --outputFile=\"coverage/report.json\" --forceExit",
"test:coverage:jest": "NODE_ENV=test PORT=4243 jest --silent --ci --json --coverage --testLocationInResults --outputFile=\"report.json\" --forceExit",
"test:updateSnapshot": "NODE_ENV=test PORT=4243 jest --updateSnapshot",
"seed:setup": "ts-node --compilerOptions '{\"strictNullChecks\": false}' src/test/e2e/seed/segment.seed.ts",
"seed:serve": "UNLEASH_DATABASE_NAME=unleash_test UNLEASH_DATABASE_SCHEMA=seed yarn run start:dev",
"clean": "del-cli --force dist",
Expand All @@ -81,7 +81,7 @@
"automock": false,
"maxWorkers": 4,
"testTimeout": 20000,
"globalSetup": "./scripts/jest-setup.js",
"globalSetup": "./scripts/jest-setup.ts",
"transform": {
"^.+\\.tsx?$": [
"@swc/jest"
Expand Down
3 changes: 0 additions & 3 deletions scripts/jest-setup.js

This file was deleted.

35 changes: 35 additions & 0 deletions scripts/jest-setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { Client, type ClientConfig } from 'pg';
import { migrateDb } from '../src/migrator';
import { getDbConfig } from '../src/test/e2e/helpers/database-config';

let initializationPromise: Promise<void> | null = null;
const initializeTemplateDb = (db: ClientConfig): Promise<void> => {
if (!initializationPromise) {
initializationPromise = (async () => {
const testDBTemplateName = process.env.TEST_DB_TEMPLATE_NAME;
const client = new Client(db);
await client.connect();
console.log(`Initializing template database ${testDBTemplateName}`);
// code to clean up, but only on next run, we could do it at tear down... but is it really needed?
// const result = await client.query(`select datname from pg_database where datname like 'unleashtestdb_%';`)
// result.rows.forEach(async (row: any) => {
// console.log(`Dropping test database ${row.datname}`);
// await client.query(`DROP DATABASE ${row.datname}`);
// });
await client.query(`DROP DATABASE IF EXISTS ${testDBTemplateName}`);
await client.query(`CREATE DATABASE ${testDBTemplateName}`);
await client.end();
await migrateDb({
db: { ...db, database: testDBTemplateName },
} as any);
console.log(`Template database ${testDBTemplateName} migrated`);
})();
}
return initializationPromise;
};

export default async function globalSetup() {
process.env.TZ = 'UTC';
process.env.TEST_DB_TEMPLATE_NAME = 'unleash_template_db';
await initializeTemplateDb(getDbConfig());
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test(`Should indicate change request enabled status`, async () => {
// change request enabled in enabled environment
await db.rawDatabase('change_request_settings').insert({
project: 'default',
environment: 'default',
environment: 'development',
required_approvals: 1,
});
const enabledStatus =
Expand All @@ -41,7 +41,7 @@ test(`Should indicate change request enabled status`, async () => {
// change request enabled in disabled environment
await db.stores.projectStore.deleteEnvironmentForProject(
'default',
'default',
'development',
);
const disabledStatus =
await readModel.isChangeRequestsEnabledForProject('default');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ let db: ITestDb;
let eventStore: IEventStore;

beforeAll(async () => {
db = await dbInit('dependent_features', getLogger);
db = await dbInit('dependent_features', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithCustomConfig(
db.stores,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ let eventBus: EventEmitter;
let featureLifecycleReadModel: IFeatureLifecycleReadModel;

beforeAll(async () => {
db = await dbInit('feature_lifecycle', getLogger);
db = await dbInit('feature_lifecycle', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithAuth(
db.stores,
{
Expand Down
4 changes: 3 additions & 1 deletion src/lib/features/feature-search/feature.search.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ let db: ITestDb;
let stores: IUnleashStores;

beforeAll(async () => {
db = await dbInit('feature_search', getLogger);
db = await dbInit('feature_search', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithAuth(
db.stores,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ let app: IUnleashTest;
let db: ITestDb;

beforeAll(async () => {
db = await dbInit('archive_test_serial', getLogger);
db = await dbInit('archive_test_serial', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithCustomConfig(
db.stores,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ import {
setupAppWithCustomConfig,
} from '../../../../test/e2e/helpers/test-helper';
import getLogger from '../../../../test/fixtures/no-logger';
import type { IUnleashOptions } from '../../../internals';

let app: IUnleashTest;
let db: ITestDb;

const setupLastSeenAtTest = async (featureName: string) => {
await app.createFeature(featureName);

await insertLastSeenAt(featureName, db.rawDatabase, 'default');
await insertLastSeenAt(featureName, db.rawDatabase, 'development');
await insertLastSeenAt(featureName, db.rawDatabase, 'production');
};

beforeAll(async () => {
const config = {
const config: Partial<IUnleashOptions> = {
experimental: {
flags: {
strictSchemaValidation: true,
Expand All @@ -34,29 +34,6 @@ beforeAll(async () => {
config,
);
app = await setupAppWithCustomConfig(db.stores, config, db.rawDatabase);

await db.stores.environmentStore.create({
name: 'development',
type: 'development',
sortOrder: 1,
enabled: true,
});

await db.stores.environmentStore.create({
name: 'production',
type: 'production',
sortOrder: 2,
enabled: true,
});

await app.services.projectService.addEnvironmentToProject(
'default',
'development',
);
await app.services.projectService.addEnvironmentToProject(
'default',
'production',
);
});

afterAll(async () => {
Expand All @@ -67,7 +44,7 @@ afterAll(async () => {
test('should return last seen at per env for /api/admin/features', async () => {
await app.createFeature('lastSeenAtPerEnv');

await insertLastSeenAt('lastSeenAtPerEnv', db.rawDatabase, 'default');
await insertLastSeenAt('lastSeenAtPerEnv', db.rawDatabase, 'development');

const response = await app.request
.get('/api/admin/projects/default/features')
Expand All @@ -94,10 +71,7 @@ test('response should include last seen at per environment for multiple environm

const featureEnvironments = body.features[1].environments;

const [def, development, production] = featureEnvironments;

expect(def.name).toBe('default');
expect(def.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
const [development, production] = featureEnvironments;

expect(development.name).toBe('development');
expect(development.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
Expand All @@ -117,10 +91,7 @@ test('response should include last seen at per environment for multiple environm
const { body } = await app.request.get(`/api/admin/archive/features`);

const featureEnvironments = body.features[0].environments;
const [def, development, production] = featureEnvironments;

expect(def.name).toBe('default');
expect(def.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
const [development, production] = featureEnvironments;

expect(development.name).toBe('development');
expect(development.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
Expand All @@ -142,10 +113,7 @@ test('response should include last seen at per environment for multiple environm
);

const featureEnvironments = body.features[0].environments;
const [def, development, production] = featureEnvironments;

expect(def.name).toBe('default');
expect(def.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
const [development, production] = featureEnvironments;

expect(development.name).toBe('development');
expect(development.lastSeenAt).toEqual('2023-10-01T12:34:56.000Z');
Expand All @@ -163,13 +131,6 @@ test('response should include last seen at per environment correctly for a singl
await setupLastSeenAtTest(`${featureName}4`);
await setupLastSeenAtTest(`${featureName}5`);

await insertLastSeenAt(
featureName,
db.rawDatabase,
'default',
'2023-08-01T12:30:56.000Z',
);

await insertLastSeenAt(
featureName,
db.rawDatabase,
Expand All @@ -189,10 +150,6 @@ test('response should include last seen at per environment correctly for a singl
.expect(200);

const expected = [
{
name: 'default',
lastSeenAt: '2023-08-01T12:30:56.000Z',
},
{
name: 'development',
lastSeenAt: '2023-08-01T12:30:56.000Z',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ beforeAll(async () => {
db = await dbInit(
'feature_toggle_service_v2_service_serial',
config.getLogger,
{ dbInitMethod: 'legacy' as const },
);
unleashConfig = config;
stores = db.stores;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ let app: IUnleashTest;
let db: ITestDb;

beforeAll(async () => {
db = await dbInit('feature_strategy_auth_api_serial', getLogger);
db = await dbInit('feature_strategy_auth_api_serial', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithAuth(
db.stores,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ const updateStrategy = async (
};

beforeAll(async () => {
db = await dbInit('feature_strategy_api_serial', getLogger);
db = await dbInit('feature_strategy_api_serial', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithCustomConfig(
db.stores,
{
Expand Down
4 changes: 3 additions & 1 deletion src/lib/features/frontend-api/frontend-api.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ let app: IUnleashTest;
let db: ITestDb;
let frontendApiService: FrontendApiService;
beforeAll(async () => {
db = await dbInit('frontend_api', getLogger);
db = await dbInit('frontend_api', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithAuth(
db.stores,
{
Expand Down
12 changes: 0 additions & 12 deletions src/lib/features/instance-stats/getProductionChanges.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ const noEnvironmentEvent = (days: number) => {

beforeAll(async () => {
db = await dbInit('product_changes_serial', getLogger);
await db.rawDatabase('environments').insert({
name: 'production',
type: 'production',
enabled: true,
protected: false,
});
getProductionChanges = createGetProductionChanges(db.rawDatabase);
});

Expand Down Expand Up @@ -136,12 +130,6 @@ test('five events per day should be counted correctly', async () => {
});

test('Events posted to a non production environment should not be included in count', async () => {
await db.rawDatabase('environments').insert({
name: 'development',
type: 'development',
enabled: true,
protected: false,
});
await db.rawDatabase
.table('events')
.insert(mockRawEventDaysAgo(1, 'development'));
Expand Down
6 changes: 0 additions & 6 deletions src/lib/features/metrics/instance/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,6 @@ describe('bulk metrics', () => {
enableApiToken: true,
},
});
await authed.db('environments').insert({
name: 'development',
sort_order: 5000,
type: 'development',
enabled: true,
});
const clientToken =
await authed.services.apiTokenService.createApiTokenWithProjects({
tokenName: 'bulk-metrics-test',
Expand Down
4 changes: 3 additions & 1 deletion src/lib/features/playground/advanced-playground.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ let app: IUnleashTest;
let db: ITestDb;

beforeAll(async () => {
db = await dbInit('advanced_playground', getLogger);
db = await dbInit('advanced_playground', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithCustomConfig(
db.stores,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ let eventService: EventService;

beforeAll(async () => {
const config = createTestConfig();
db = await dbInit('environment_service_serial', config.getLogger);
db = await dbInit('environment_service_serial', config.getLogger, {
dbInitMethod: 'legacy' as const,
});
stores = db.stores;
eventService = createEventsService(db.rawDatabase, config);
service = new EnvironmentService(stores, config, eventService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ let app: IUnleashTest;
let db: ITestDb;

beforeAll(async () => {
db = await dbInit('project_environments_api_serial', getLogger);
db = await dbInit('project_environments_api_serial', getLogger, {
dbInitMethod: 'legacy' as const,
});
app = await setupAppWithCustomConfig(
db.stores,
{
Expand Down
11 changes: 2 additions & 9 deletions src/lib/features/project/project-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ test('A newly created project only gets connected to enabled environments', asyn
await projectService.createProject(project, user, auditUser);
const connectedEnvs =
await db.stores.projectStore.getEnvironmentsForProject(project.id);
expect(connectedEnvs).toHaveLength(2); // default, connection_test
expect(connectedEnvs).toHaveLength(1); // connection_test
expect(
connectedEnvs.some((e) => e.environment === enabledEnv),
).toBeTruthy();
Expand Down Expand Up @@ -1321,7 +1321,6 @@ test('should have environments sorted in order', async () => {
await db.stores.projectStore.getEnvironmentsForProject(project.id);

expect(connectedEnvs.map((e) => e.environment)).toEqual([
'default',
first,
second,
third,
Expand Down Expand Up @@ -2809,13 +2808,7 @@ describe('create project with environments', () => {
disabledEnv,
];

const allEnabledEnvs = [
'QA',
'default',
'development',
'production',
'staging',
];
const allEnabledEnvs = ['QA', 'development', 'production', 'staging'];

beforeEach(async () => {
await Promise.all(
Expand Down
Loading

0 comments on commit 5e9698f

Please sign in to comment.