From a79a76f497520c8513f09cc6dd8350cdba88bbbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Thu, 23 Mar 2023 16:31:05 +0100 Subject: [PATCH] refactor: test composition and other error codes (#3348) ## About the changes Small refactor to showcase how to use [composition to validate different aspects of the response](https://github.com/Unleash/unleash/pull/3348/files#diff-ee4c1bd501b1195162b7a85ed6be348a665288f871abc8e74f64d94361213f9eR361-R367) and checking [different status codes](https://github.com/Unleash/unleash/pull/3348/files#diff-4044a5da3280ef76960bbffd5f36eccb395ac319fe58c4d59ef68a878cbb1a5dR95) --- .../export-import.e2e.test.ts | 65 +++------ .../e2e/api/admin/feature-archive.e2e.test.ts | 138 ++++++------------ .../api/admin/project/features.e2e.test.ts | 98 ++++--------- src/test/e2e/api/client/segment.e2e.test.ts | 6 +- src/test/e2e/helpers/test-helper.ts | 91 +++++++++++- 5 files changed, 185 insertions(+), 213 deletions(-) diff --git a/src/lib/features/export-import-toggles/export-import.e2e.test.ts b/src/lib/features/export-import-toggles/export-import.e2e.test.ts index 6c77430cdb..924fc8eaa2 100644 --- a/src/lib/features/export-import-toggles/export-import.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import.e2e.test.ts @@ -112,12 +112,7 @@ const createProjects = async (projects: string[] = [DEFAULT_PROJECT]) => { id: project, mode: 'open' as const, }); - await app.request - .post(`/api/admin/projects/${project}/environments`) - .send({ - environment: DEFAULT_ENV, - }) - .expect(200); + await app.linkProjectToEnvironment(project, DEFAULT_ENV); } }; @@ -127,18 +122,6 @@ const createSegment = (postData: UpsertSegmentSchema): Promise => { }); }; -const createContextField = async (contextField: IContextFieldDto) => { - await app.createContextField(contextField); -}; - -const createFeature = async (featureName: string) => { - await app.createFeature(featureName); -}; - -const archiveFeature = async (featureName: string) => { - await app.archiveFeature(featureName); -}; - const unArchiveFeature = async (featureName: string) => { await app.request .post(`/api/admin/archive/revive/${featureName}`) @@ -176,7 +159,7 @@ beforeEach(async () => { await environmentStore.deleteAll(); await contextFieldStore.deleteAll(); - await createContextField({ name: 'appName' }); + await app.createContextField({ name: 'appName' }); }); afterAll(async () => { @@ -475,18 +458,6 @@ test('returns no features, when no feature was requested', async () => { expect(body.features).toHaveLength(0); }); -const importToggles = ( - importPayload: ImportTogglesSchema, - status = 200, - expect: (response) => void = () => {}, -) => - app.request - .post('/api/admin/features-batch/import') - .send(importPayload) - .set('Content-Type', 'application/json') - .expect(status) - .expect(expect); - const defaultFeature = 'first_feature'; const variants: VariantsSchema = [ @@ -610,7 +581,7 @@ const validateImport = (importPayload: ImportTogglesSchema, status = 200) => test('import features to existing project and environment', async () => { await createProjects(); - await importToggles(defaultImportPayload); + await app.importToggles(defaultImportPayload); const { body: importedFeature } = await getFeature(defaultFeature); expect(importedFeature).toMatchObject({ @@ -645,8 +616,8 @@ test('import features to existing project and environment', async () => { test('importing same JSON should work multiple times in a row', async () => { await createProjects(); - await importToggles(defaultImportPayload); - await importToggles(defaultImportPayload); + await app.importToggles(defaultImportPayload); + await app.importToggles(defaultImportPayload); const { body: importedFeature } = await getFeature(defaultFeature); expect(importedFeature).toMatchObject({ @@ -681,7 +652,7 @@ test('reject import with unknown context fields', async () => { name: 'ContextField1', legalValues: [{ value: 'Value1', description: '' }], }; - await createContextField(contextField); + await app.createContextField(contextField); const importPayloadWithContextFields: ImportTogglesSchema = { ...defaultImportPayload, data: { @@ -695,7 +666,10 @@ test('reject import with unknown context fields', async () => { }, }; - const { body } = await importToggles(importPayloadWithContextFields, 400); + const { body } = await app.importToggles( + importPayloadWithContextFields, + 400, + ); expect(body).toMatchObject({ details: [ @@ -718,7 +692,10 @@ test('reject import with unsupported strategies', async () => { }, }; - const { body } = await importToggles(importPayloadWithContextFields, 400); + const { body } = await app.importToggles( + importPayloadWithContextFields, + 400, + ); expect(body).toMatchObject({ details: [ @@ -741,10 +718,10 @@ test('validate import data', async () => { legalValues: [{ value: 'new_value' }], }; - await createFeature(defaultFeature); - await archiveFeature(defaultFeature); + await app.createFeature(defaultFeature); + await app.archiveFeature(defaultFeature); - await createContextField(contextField); + await app.createContextField(contextField); const importPayloadWithContextFields: ImportTogglesSchema = { ...defaultImportPayload, data: { @@ -801,7 +778,7 @@ test('should create new context', async () => { }, }; - await importToggles(importPayloadWithContextFields, 200); + await app.importToggles(importPayloadWithContextFields); const { body } = await getContextField(context.name); expect(body).toMatchObject(context); @@ -809,11 +786,11 @@ test('should create new context', async () => { test('should not import archived features tags', async () => { await createProjects(); - await importToggles(defaultImportPayload); + await app.importToggles(defaultImportPayload); - await archiveFeature(defaultFeature); + await app.archiveFeature(defaultFeature); - await importToggles({ + await app.importToggles({ ...defaultImportPayload, data: { ...defaultImportPayload.data, diff --git a/src/test/e2e/api/admin/feature-archive.e2e.test.ts b/src/test/e2e/api/admin/feature-archive.e2e.test.ts index fd88f49fad..9b835db7ff 100644 --- a/src/test/e2e/api/admin/feature-archive.e2e.test.ts +++ b/src/test/e2e/api/admin/feature-archive.e2e.test.ts @@ -1,19 +1,13 @@ -import { setupAppWithCustomConfig } from '../../helpers/test-helper'; -import dbInit from '../../helpers/database-init'; +import { + IUnleashTest, + setupAppWithCustomConfig, +} from '../../helpers/test-helper'; +import dbInit, { ITestDb } from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; import { DEFAULT_PROJECT } from '../../../../lib/types'; -let app; -let db; - -const createFeatureToggle = ( - featureName: string, - project = DEFAULT_PROJECT, -) => { - return app.request.post(`/api/admin/projects/${project}/features`).send({ - name: featureName, - }); -}; +let app: IUnleashTest; +let db: ITestDb; beforeAll(async () => { db = await dbInit('archive_serial', getLogger); @@ -25,78 +19,42 @@ beforeAll(async () => { }, }, }); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureX', - description: 'the #1 feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureY', - description: 'soon to be the #1 feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureZ', - description: 'terrible feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureArchivedX', - description: 'the #1 feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.archiveToggle( - 'featureArchivedX', - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureArchivedY', - description: 'soon to be the #1 feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.archiveToggle( - 'featureArchivedY', - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'featureArchivedZ', - description: 'terrible feature', - }, - 'test', - ); - await app.services.featureToggleServiceV2.archiveToggle( - 'featureArchivedZ', - 'test', - ); - await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', - { - name: 'feature.with.variants', - description: 'A feature toggle with variants', - variants: [ - { name: 'control', weight: 50 }, - { name: 'new', weight: 50 }, - ], - }, - 'test', - ); + await app.createFeature({ + name: 'featureX', + description: 'the #1 feature', + }); + await app.createFeature({ + name: 'featureY', + description: 'soon to be the #1 feature', + }); + await app.createFeature({ + name: 'featureZ', + description: 'terrible feature', + }); + await app.createFeature({ + name: 'featureArchivedX', + description: 'the #1 feature', + }); + await app.archiveFeature('featureArchivedX'); + + await app.createFeature({ + name: 'featureArchivedY', + description: 'soon to be the #1 feature', + }); + await app.archiveFeature('featureArchivedY'); + await app.createFeature({ + name: 'featureArchivedZ', + description: 'terrible feature', + }); + await app.archiveFeature('featureArchivedZ'); + await app.createFeature({ + name: 'feature.with.variants', + description: 'A feature toggle with variants', + variants: [ + { name: 'control', weight: 50 }, + { name: 'new', weight: 50 }, + ], + }); }); afterAll(async () => { @@ -138,10 +96,8 @@ test('revives a feature by name', async () => { test('archived feature is not accessible via /features/:featureName', async () => { expect.assertions(0); - return app.request - .get('/api/admin/features/featureArchivedZ') - .set('Content-Type', 'application/json') - .expect(404); + await app.getFeatures('featureArchivedZ', 404); + await app.getProjectFeatures('default', 'featureArchivedZ', 404); }); test('must set name when reviving toggle', async () => { @@ -267,8 +223,8 @@ test('Should be able to bulk archive features', async () => { const featureName1 = 'archivedFeature1'; const featureName2 = 'archivedFeature2'; - await createFeatureToggle(featureName1); - await createFeatureToggle(featureName2); + await app.createFeature(featureName1); + await app.createFeature(featureName2); await app.request .post(`/api/admin/projects/${DEFAULT_PROJECT}/archive`) diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 1ad98fa28c..23fab8a943 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -33,15 +33,6 @@ const sortOrderFirst = 0; const sortOrderSecond = 10; const sortOrderDefault = 9999; -const createFeatureToggle = ( - featureName: string, - project = DEFAULT_PROJECT, -) => { - return app.request.post(`/api/admin/projects/${project}/features`).send({ - name: featureName, - }); -}; - const createSegment = async (segmentName: string) => { const segment = await app.services.segmentService.create( { @@ -317,6 +308,7 @@ test('Disconnecting environment from project, removes environment from features test('Can enable/disable environment for feature with strategies', async () => { const envName = 'enable-feature-environment'; const featureName = 'com.test.enable.environment'; + const project = 'default'; // Create environment await db.stores.environmentStore.create({ name: envName, @@ -324,29 +316,22 @@ test('Can enable/disable environment for feature with strategies', async () => { }); // Connect environment to project await app.request - .post('/api/admin/projects/default/environments') + .post(`/api/admin/projects/${project}/environments`) .send({ environment: envName, }) .expect(200); // Create feature - await app.request - .post('/api/admin/projects/default/features') - .send({ - name: featureName, - }) - .set('Content-Type', 'application/json') - .expect(201) - .expect((res) => { - expect(res.body.name).toBe(featureName); - expect(res.body.createdAt).toBeTruthy(); - }); + await app.createFeature(featureName).expect((res) => { + expect(res.body.name).toBe(featureName); + expect(res.body.createdAt).toBeTruthy(); + }); // Add strategy to it await app.request .post( - `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`, + `/api/admin/projects/${project}/features/${featureName}/environments/${envName}/strategies`, ) .send({ name: 'default', @@ -357,38 +342,30 @@ test('Can enable/disable environment for feature with strategies', async () => { .expect(200); await app.request .post( - `/api/admin/projects/default/features/${featureName}/environments/${envName}/on`, + `/api/admin/projects/${project}/features/${featureName}/environments/${envName}/on`, ) .set('Content-Type', 'application/json') .expect(200); - await app.request - .get(`/api/admin/projects/default/features/${featureName}`) - .expect(200) - .expect('Content-Type', /json/) - .expect((res) => { - const enabledFeatureEnv = res.body.environments.find( - (e) => e.name === 'enable-feature-environment', - ); - expect(enabledFeatureEnv).toBeTruthy(); - expect(enabledFeatureEnv.enabled).toBe(true); - }); + await app.getProjectFeatures(project, featureName).expect((res) => { + const enabledFeatureEnv = res.body.environments.find( + (e) => e.name === 'enable-feature-environment', + ); + expect(enabledFeatureEnv).toBeTruthy(); + expect(enabledFeatureEnv.enabled).toBe(true); + }); await app.request .post( - `/api/admin/projects/default/features/${featureName}/environments/${envName}/off`, + `/api/admin/projects/${project}/features/${featureName}/environments/${envName}/off`, ) .send({}) .expect(200); - await app.request - .get(`/api/admin/projects/default/features/${featureName}`) - .expect(200) - .expect('Content-Type', /json/) - .expect((res) => { - const disabledFeatureEnv = res.body.environments.find( - (e) => e.name === 'enable-feature-environment', - ); - expect(disabledFeatureEnv).toBeTruthy(); - expect(disabledFeatureEnv.enabled).toBe(false); - }); + await app.getProjectFeatures(project, featureName).expect((res) => { + const disabledFeatureEnv = res.body.environments.find( + (e) => e.name === 'enable-feature-environment', + ); + expect(disabledFeatureEnv).toBeTruthy(); + expect(disabledFeatureEnv.enabled).toBe(false); + }); }); test("Trying to get a project that doesn't exist yields 404", async () => { @@ -1975,12 +1952,7 @@ test('Should not allow changing project to target project without the same enabl 'default', ); - await app.request - .post(`/api/admin/projects/${project}/features`) - .send({ - name: featureName, - }) - .expect(201); + await app.createFeature(featureName, project); await app.request .post( `/api/admin/projects/${project}/features/${featureName}/environments/default/strategies`, @@ -2060,12 +2032,7 @@ test('Should allow changing project to target project with the same enabled envi ); await db.stores.projectStore.addEnvironmentToProject(targetProject, inBoth); - await app.request - .post(`/api/admin/projects/${project}/features`) - .send({ - name: featureName, - }) - .expect(201); + await app.createFeature(featureName, project); await app.request .post( `/api/admin/projects/${project}/features/${featureName}/environments/default/strategies`, @@ -2122,12 +2089,7 @@ test('Should allow changing project to target project with the same enabled envi test(`a feature's variants should be sorted by name in increasing order`, async () => { const featureName = 'variants.are.sorted'; const project = 'default'; - await app.request - .post(`/api/admin/projects/${project}/features`) - .send({ - name: featureName, - }) - .expect(201); + await app.createFeature(featureName, project); const newVariants: IVariant[] = [ { @@ -2652,7 +2614,7 @@ test('should return strategies in correct order when new strategies are added', test('should create a strategy with segments', async () => { const feature = { name: uuidv4(), impressionData: false }; - await createFeatureToggle(feature.name); + await app.createFeature(feature.name); const segment = await createSegment('segmentOne'); const { body: strategyOne } = await createStrategy(feature.name, { name: 'default', @@ -2700,7 +2662,7 @@ test('should create a strategy with segments', async () => { test('should add multiple segments to a strategy', async () => { const feature = { name: uuidv4(), impressionData: false }; - await createFeatureToggle(feature.name); + await app.createFeature(feature.name); const segment = await createSegment('seg1'); const segmentTwo = await createSegment('seg2'); const segmentThree = await createSegment('seg3'); @@ -2834,8 +2796,8 @@ test('Should batch stale features', async () => { const staledFeatureName1 = 'staledFeature1'; const staledFeatureName2 = 'staledFeature2'; - await createFeatureToggle(staledFeatureName1); - await createFeatureToggle(staledFeatureName2); + await app.createFeature(staledFeatureName1); + await app.createFeature(staledFeatureName2); await app.request .post(`/api/admin/projects/${DEFAULT_PROJECT}/stale`) diff --git a/src/test/e2e/api/client/segment.e2e.test.ts b/src/test/e2e/api/client/segment.e2e.test.ts index baef701965..416f3e43da 100644 --- a/src/test/e2e/api/client/segment.e2e.test.ts +++ b/src/test/e2e/api/client/segment.e2e.test.ts @@ -97,11 +97,7 @@ const createFeatureToggle = async ( { status: 200 }, ], ): Promise => { - await app.request - .post(`/api/admin/projects/${project}/features`) - .send(feature) - .expect(expectStatusCode); - + await app.createFeature(feature, project, expectStatusCode); let processed = 0; for (const strategy of strategies) { const { body, status } = await app.request diff --git a/src/test/e2e/helpers/test-helper.ts b/src/test/e2e/helpers/test-helper.ts index 0ca7725d98..74ac43a4fc 100644 --- a/src/test/e2e/helpers/test-helper.ts +++ b/src/test/e2e/helpers/test-helper.ts @@ -10,6 +10,8 @@ import { DEFAULT_PROJECT, IUnleashStores } from '../../../lib/types'; import { IUnleashServices } from '../../../lib/types/services'; import { Db } from '../../../lib/db/db'; import { IContextFieldDto } from 'lib/types/stores/context-field-store'; +import { DEFAULT_ENV } from '../../../lib/util'; +import { CreateFeatureSchema, ImportTogglesSchema } from '../../../lib/openapi'; process.env.NODE_ENV = 'test'; @@ -20,21 +22,47 @@ export interface IUnleashTest extends IUnleashHttpAPI { config: IUnleashConfig; } +/** + * This is a collection of API helpers. The response code is optional, and should default to the success code for the request. + * + * All functions return a supertest.Test object, which can be used to compose more assertions on the response. + */ export interface IUnleashHttpAPI { createFeature( - name: string, + feature: string | CreateFeatureSchema, project?: string, expectedResponseCode?: number, ): supertest.Test; + + getFeatures(name?: string, expectedResponseCode?: number): supertest.Test; + + getProjectFeatures( + project: string, + name?: string, + expectedResponseCode?: number, + ): supertest.Test; + archiveFeature( name: string, project?: string, expectedResponseCode?: number, ): supertest.Test; + createContextField( contextField: IContextFieldDto, expectedResponseCode?: number, ): supertest.Test; + + linkProjectToEnvironment( + project: string, + environment: string, + expectedResponseCode?: number, + ): supertest.Test; + + importToggles( + importPayload: ImportTogglesSchema, + expectedResponseCode?: number, + ): supertest.Test; } function httpApis( @@ -45,15 +73,44 @@ function httpApis( return { createFeature: ( - name: string, + feature: string | CreateFeatureSchema, project: string = DEFAULT_PROJECT, expectedResponseCode: number = 201, ) => { + let body = feature; + if (typeof feature === 'string') { + body = { + name: feature, + }; + } return request .post(`${base}/api/admin/projects/${project}/features`) - .send({ - name, - }) + .send(body) + .set('Content-Type', 'application/json') + .expect(expectedResponseCode); + }, + + getFeatures( + name?: string, + expectedResponseCode: number = 200, + ): supertest.Test { + const featuresUrl = `/api/admin/features${name ? `/${name}` : ''}`; + return request + .get(featuresUrl) + .set('Content-Type', 'application/json') + .expect(expectedResponseCode); + }, + + getProjectFeatures( + project: string = DEFAULT_PROJECT, + name?: string, + expectedResponseCode: number = 200, + ): supertest.Test { + const featuresUrl = `/api/admin/projects/${project}/features${ + name ? `/${name}` : '' + }`; + return request + .get(featuresUrl) .set('Content-Type', 'application/json') .expect(expectedResponseCode); }, @@ -80,6 +137,30 @@ function httpApis( .send(contextField) .expect(expectedResponseCode); }, + + linkProjectToEnvironment( + project: string, + environment: string = DEFAULT_ENV, + expectedResponseCode: number = 200, + ): supertest.Test { + return request + .post(`${base}/api/admin/projects/${project}/environments`) + .send({ + environment, + }) + .expect(expectedResponseCode); + }, + + importToggles( + importPayload: ImportTogglesSchema, + expectedResponseCode: number = 200, + ): supertest.Test { + return request + .post('/api/admin/features-batch/import') + .send(importPayload) + .set('Content-Type', 'application/json') + .expect(expectedResponseCode); + }, }; }