diff --git a/src/lib/services/client-metrics/instance-service.test.ts b/src/lib/services/client-metrics/instance-service.test.ts index dcb53880dc39..e706b7a48881 100644 --- a/src/lib/services/client-metrics/instance-service.test.ts +++ b/src/lib/services/client-metrics/instance-service.test.ts @@ -5,45 +5,11 @@ import FakeEventStore from '../../../test/fixtures/fake-event-store'; import { createTestConfig } from '../../../test/config/test-config'; import { FakePrivateProjectChecker } from '../../features/private-project/fakePrivateProjectChecker'; -/** - * A utility to wait for any pending promises in the test subject code. - * For instance, if the test needs to wait for a timeout/interval handler, - * and that handler does something async, advancing the timers is not enough: - * We have to explicitly wait for the second promise. - * For more info, see https://stackoverflow.com/a/51045733/2868829 - * - * Usage in test code after advancing timers, but before making assertions: - * - * test('hello', async () => { - * jest.useFakeTimers('modern'); - * - * // Schedule a timeout with a callback that does something async - * // before calling our spy - * const spy = jest.fn(); - * setTimeout(async () => { - * await Promise.resolve(); - * spy(); - * }, 1000); - * - * expect(spy).not.toHaveBeenCalled(); - * - * jest.advanceTimersByTime(1500); - * await flushPromises(); // this is required to make it work! - * - * expect(spy).toHaveBeenCalledTimes(1); - * - * jest.useRealTimers(); - * }); - */ -function flushPromises() { - return Promise.resolve(setImmediate); -} let config; beforeAll(() => { config = createTestConfig({}); }); test('Multiple registrations of same appname and instanceid within same time period should only cause one registration', async () => { - jest.useFakeTimers(); const appStoreSpy = jest.fn(); const bulkSpy = jest.fn(); const clientApplicationsStore: any = { @@ -75,8 +41,8 @@ test('Multiple registrations of same appname and instanceid within same time per await clientMetrics.registerClient(client1, '127.0.0.1'); await clientMetrics.registerClient(client1, '127.0.0.1'); await clientMetrics.registerClient(client1, '127.0.0.1'); - jest.advanceTimersByTime(7000); - await flushPromises(); + + await clientMetrics.bulkAdd(); // in prod called by a SchedulerService expect(appStoreSpy).toHaveBeenCalledTimes(1); expect(bulkSpy).toHaveBeenCalledTimes(1); @@ -93,7 +59,6 @@ test('Multiple registrations of same appname and instanceid within same time per }); test('Multiple unique clients causes multiple registrations', async () => { - jest.useFakeTimers(); const appStoreSpy = jest.fn(); const bulkSpy = jest.fn(); const clientApplicationsStore: any = { @@ -136,16 +101,14 @@ test('Multiple unique clients causes multiple registrations', async () => { await clientMetrics.registerClient(client2, '127.0.0.1'); await clientMetrics.registerClient(client2, '127.0.0.1'); - jest.advanceTimersByTime(7000); - await flushPromises(); + await clientMetrics.bulkAdd(); // in prod called by a SchedulerService const registrations = appStoreSpy.mock.calls[0][0]; expect(registrations.length).toBe(2); - jest.useRealTimers(); }); + test('Same client registered outside of dedup interval will be registered twice', async () => { - jest.useFakeTimers(); const appStoreSpy = jest.fn(); const bulkSpy = jest.fn(); const clientApplicationsStore: any = { @@ -155,8 +118,6 @@ test('Same client registered outside of dedup interval will be registered twice' bulkUpsert: bulkSpy, }; - const bulkInterval = secondsToMilliseconds(2); - const clientMetrics = new ClientInstanceService( { clientMetricsStoreV2: null, @@ -168,7 +129,6 @@ test('Same client registered outside of dedup interval will be registered twice' }, config, new FakePrivateProjectChecker(), - bulkInterval, ); const client1 = { appName: 'test_app', @@ -181,14 +141,13 @@ test('Same client registered outside of dedup interval will be registered twice' await clientMetrics.registerClient(client1, '127.0.0.1'); await clientMetrics.registerClient(client1, '127.0.0.1'); - jest.advanceTimersByTime(3000); + await clientMetrics.bulkAdd(); // in prod called by a SchedulerService await clientMetrics.registerClient(client1, '127.0.0.1'); await clientMetrics.registerClient(client1, '127.0.0.1'); await clientMetrics.registerClient(client1, '127.0.0.1'); - jest.advanceTimersByTime(3000); - await flushPromises(); + await clientMetrics.bulkAdd(); // in prod called by a SchedulerService expect(appStoreSpy).toHaveBeenCalledTimes(2); expect(bulkSpy).toHaveBeenCalledTimes(2); @@ -198,11 +157,9 @@ test('Same client registered outside of dedup interval will be registered twice' expect(firstRegistrations.appName).toBe(secondRegistrations.appName); expect(firstRegistrations.instanceId).toBe(secondRegistrations.instanceId); - jest.useRealTimers(); }); test('No registrations during a time period will not call stores', async () => { - jest.useFakeTimers(); const appStoreSpy = jest.fn(); const bulkSpy = jest.fn(); const clientApplicationsStore: any = { @@ -211,7 +168,7 @@ test('No registrations during a time period will not call stores', async () => { const clientInstanceStore: any = { bulkUpsert: bulkSpy, }; - new ClientInstanceService( + const clientMetrics = new ClientInstanceService( { clientMetricsStoreV2: null, strategyStore: null, @@ -223,8 +180,9 @@ test('No registrations during a time period will not call stores', async () => { config, new FakePrivateProjectChecker(), ); - jest.advanceTimersByTime(6000); + + await clientMetrics.bulkAdd(); // in prod called by a SchedulerService + expect(appStoreSpy).toHaveBeenCalledTimes(0); expect(bulkSpy).toHaveBeenCalledTimes(0); - jest.useRealTimers(); }); diff --git a/src/lib/services/client-metrics/instance-service.ts b/src/lib/services/client-metrics/instance-service.ts index 1e1acd4ac7f4..7b31d891a615 100644 --- a/src/lib/services/client-metrics/instance-service.ts +++ b/src/lib/services/client-metrics/instance-service.ts @@ -45,10 +45,6 @@ export default class ClientInstanceService { private flagResolver: IFlagResolver; - private bulkInterval: number; - - private announcementInterval: number; - constructor( { clientMetricsStoreV2, @@ -71,8 +67,6 @@ export default class ClientInstanceService { flagResolver, }: Pick, privateProjectChecker: IPrivateProjectChecker, - bulkInterval = secondsToMilliseconds(5), - announcementInterval = minutesToMilliseconds(5), ) { this.clientMetricsStoreV2 = clientMetricsStoreV2; this.strategyStore = strategyStore; @@ -85,9 +79,6 @@ export default class ClientInstanceService { this.logger = getLogger( '/services/client-metrics/client-instance-service.ts', ); - - this.bulkInterval = bulkInterval; - this.announcementInterval = announcementInterval; } public async registerInstance( diff --git a/src/test/e2e/services/client-metrics-service.e2e.test.ts b/src/test/e2e/services/client-metrics-service.e2e.test.ts index 2f258e19a8f4..e9088285dc82 100644 --- a/src/test/e2e/services/client-metrics-service.e2e.test.ts +++ b/src/test/e2e/services/client-metrics-service.e2e.test.ts @@ -12,7 +12,7 @@ const { APPLICATION_CREATED } = require('../../../lib/types/events'); let stores; let db; -let clientInstanceService; +let clientInstanceService: ClientInstanceService; let config: IUnleashConfig; beforeAll(async () => { db = await dbInit('client_metrics_service_serial', getLogger); @@ -25,13 +25,10 @@ beforeAll(async () => { stores, config, new FakePrivateProjectChecker(), - bulkInterval, - announcementInterval, ); }); afterAll(async () => { - await clientInstanceService.destroy(); await db.destroy(); }); test('Apps registered should be announced', async () => { @@ -58,11 +55,11 @@ test('Apps registered should be announced', async () => { }; await clientInstanceService.registerClient(clientRegistration, '127.0.0.1'); await clientInstanceService.registerClient(differentClient, '127.0.0.1'); - await new Promise((res) => setTimeout(res, 1200)); + await clientInstanceService.bulkAdd(); // in prod called by a SchedulerService const first = await stores.clientApplicationsStore.getUnannounced(); expect(first.length).toBe(2); await clientInstanceService.registerClient(clientRegistration, '127.0.0.1'); - await new Promise((res) => setTimeout(res, secondsToMilliseconds(2))); + await clientInstanceService.announceUnannounced(); // in prod called by a SchedulerService const second = await stores.clientApplicationsStore.getUnannounced(); expect(second.length).toBe(0); const events = await stores.eventStore.getEvents();