From e83b9aff2f06b5e96d77f860b1bea36c7d76956e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Wed, 30 Jul 2025 15:19:25 +0200 Subject: [PATCH] Fix tests --- .../feature-toggle/feature-toggle-service.ts | 59 +++++++------------ .../tests/feature-toggle-service.e2e.test.ts | 28 ++++----- 2 files changed, 33 insertions(+), 54 deletions(-) diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 864800814d..33357cf204 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -117,7 +117,6 @@ import { sortStrategies } from '../../util/sortStrategies.js'; import type { ResourceLimitsSchema } from '../../openapi/index.js'; import type FeatureLinkService from '../feature-links/feature-link-service.js'; import type { IFeatureLink } from '../feature-links/feature-links-read-model-type.js'; -import merge from 'deepmerge'; interface IFeatureContext { featureName: string; projectId: string; @@ -126,10 +125,6 @@ interface IFeatureContext { interface IFeatureStrategyContext extends IFeatureContext { environment: string; } -function mergeAll(objects: Partial[]): T { - return merge.all(objects.filter((i) => i)); -} - export interface IGetFeatureParams { featureName: string; archived?: boolean; @@ -673,46 +668,32 @@ export class FeatureToggleService { ); } - private async defaultParameters( - projectId: string, - strategyName: string, - featureName: string, - params: IFeatureStrategy['parameters'] | undefined, - ) { - if (strategyName === 'flexibleRollout') { - if (params?.stickiness === '') { - // If stickiness is an empty string, we remove it to use the default stickiness. - delete params?.stickiness; - } - return { - rollout: params?.rollout ?? '100', - stickiness: - params?.stickiness ?? - (await this.featureStrategiesStore.getDefaultStickiness( - projectId, - )), - groupId: params?.groupId ?? featureName, - }; - } else { - /// We don't really have good defaults for the other kinds of known strategies, so return an empty map. - return {}; - } - } private async parametersWithDefaults( projectId: string, featureName: string, strategyName: string, params: IFeatureStrategy['parameters'] | undefined, ) { - return mergeAll([ - await this.defaultParameters( - projectId, - strategyName, - featureName, - params, - ), - params ?? {}, - ]); + if (strategyName === 'flexibleRollout') { + const stickiness = + params?.stickiness === undefined || params?.stickiness === '' + ? await this.featureStrategiesStore.getDefaultStickiness( + projectId, + ) + : params?.stickiness; + console.log( + `stickiness: ${stickiness} from params: ${JSON.stringify(params)}`, + ); + return { + ...params, + rollout: params?.rollout ?? '100', + stickiness, + groupId: params?.groupId ?? featureName, + }; + } else { + // We don't really have good defaults for the other kinds of known strategies, so return an empty map. + return params ?? {}; + } } private async standardizeStrategyConfig( projectId: string, diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts index 404dd9e3a1..f63e609891 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts @@ -770,6 +770,7 @@ test.each([ ], ['different rollout', { rollout: '25' }], ['empty parameters', {}], + ['extra parameters are preserved', { extra: 'value', rollout: '100' }], ])( 'Should use default parameters when creating a flexibleRollout strategy with %s', async (description, parameters: { [key: string]: any }) => { @@ -787,15 +788,12 @@ test.each([ parameters?.stickiness === '' ? defaultStickiness : (parameters?.stickiness ?? defaultStickiness); - const expectedStrategies = [ - { - parameters: { - groupId: parameters?.groupId ?? feature.name, - stickiness: expectedStickiness, - rollout: parameters?.rollout ?? '100', // default rollout - }, - }, - ]; + const expectedParameters = { + ...parameters, // expect extra parameters to be preserved + groupId: parameters?.groupId ?? feature.name, + stickiness: expectedStickiness, + rollout: parameters?.rollout ?? '100', // default rollout + }; await stores.projectStore.update({ id: projectId, name: 'stickiness-project-test', @@ -818,9 +816,9 @@ test.each([ featureName: feature.name, }); - expect(featureDB.environments[0]).toMatchObject({ - strategies: expectedStrategies, - }); + expect( + featureDB.environments[0].strategies[0].parameters, + ).toStrictEqual(expectedParameters); // Verify that updating the strategy with same data is idempotent await service.updateStrategy( @@ -833,9 +831,9 @@ test.each([ featureName: feature.name, }); - expect(featureDBAfterUpdate.environments[0]).toMatchObject({ - strategies: expectedStrategies, - }); + expect( + featureDBAfterUpdate.environments[0].strategies[0].parameters, + ).toStrictEqual(expectedParameters); }, );