From aa658d2d994992ebbc4e47cc8dfeff1ff937bd28 Mon Sep 17 00:00:00 2001 From: Brendan Burns <5751682+brendandburns@users.noreply.github.com> Date: Fri, 14 Feb 2025 21:47:23 +0000 Subject: [PATCH 1/2] Improve coverage for cache.ts --- src/cache_test.ts | 145 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 138 insertions(+), 7 deletions(-) diff --git a/src/cache_test.ts b/src/cache_test.ts index e2e0123818..f0142460f7 100644 --- a/src/cache_test.ts +++ b/src/cache_test.ts @@ -1,4 +1,4 @@ -import { deepStrictEqual, notStrictEqual, strictEqual, throws } from 'node:assert'; +import { deepStrictEqual, deepEqual, fail, notStrictEqual, strictEqual, throws } from 'node:assert'; import mock from 'ts-mockito'; import { V1Namespace, V1NamespaceList, V1ObjectMeta, V1Pod, V1PodList, V1ListMeta } from './api.js'; @@ -244,11 +244,20 @@ describe('ListWatchCache', () => { } as V1ObjectMeta, } as V1Namespace); - watchHandler('DELETED', { - metadata: { - name: 'name2', - } as V1ObjectMeta, - } as V1Namespace); + watchHandler( + 'DELETED', + { + metadata: { + name: 'name2', + resourceVersion: 'blah', + } as V1ObjectMeta, + } as V1Namespace, + { + metadata: { + resourceVersion: '54321', + }, + }, + ); const [addResult, updateResult, deleteResult] = await Promise.all([ addPromise, @@ -257,7 +266,19 @@ describe('ListWatchCache', () => { ]); deepStrictEqual(addResult.metadata, { name: 'name3' }); deepStrictEqual(updateResult.metadata, { name: 'name3', resourceVersion: 'baz' }); - deepStrictEqual(deleteResult.metadata, { name: 'name2' }); + deepStrictEqual(deleteResult.metadata, { name: 'name2', resourceVersion: 'blah' }); + strictEqual(informer.latestResourceVersion(), '54321'); + + watchHandler( + 'BOOKMARK', + {}, + { + metadata: { + resourceVersion: '5454', + }, + }, + ); + strictEqual(informer.latestResourceVersion(), '5454'); }); it('should handle change events correctly', async () => { @@ -789,6 +810,116 @@ describe('ListWatchCache', () => { strictEqual(addedList2.length, 1); }); + it('should unregister three verbs on "change"', async () => { + const fakeWatch = mock.mock(Watch); + const list: V1Namespace[] = []; + const listObj = { + metadata: { + resourceVersion: '12345', + } as V1ListMeta, + items: list, + } as V1NamespaceList; + const listFn: ListPromise = function (): Promise { + return new Promise((resolve) => { + resolve(listObj); + }); + }; + const watchCalled = new Promise((resolve) => { + mock.when( + fakeWatch.watch(mock.anything(), mock.anything(), mock.anything(), mock.anything()), + ).thenCall(resolve); + }); + const informer = new ListWatch('/some/path', mock.instance(fakeWatch), listFn); + + const changeList1: V1Namespace[] = []; + const changeToList1Fn = function (obj?: V1Namespace) { + changeList1.push(obj!); + }; + const changeList2: V1Namespace[] = []; + const changeToList2Fn = function (obj?: V1Namespace) { + changeList2.push(obj!); + }; + + informer.start(); + + await watchCalled; + const [, , watchHandler] = mock.capture(fakeWatch.watch).last(); + + informer.on('change', changeToList1Fn); + informer.on('change', changeToList2Fn); + + ['ADDED', 'DELETED', 'MODIFIED'].forEach((verb) => { + watchHandler(verb, { + metadata: { + name: 'name1', + } as V1ObjectMeta, + } as V1Namespace); + }); + strictEqual(changeList1.length, 3); + strictEqual(changeList2.length, 3); + + informer.off('change', changeToList2Fn); + + ['ADDED', 'DELETED', 'MODIFIED'].forEach((verb) => { + watchHandler(verb, { + metadata: { + name: 'name2', + } as V1ObjectMeta, + } as V1Namespace); + }); + + strictEqual(changeList1.length, 6); + strictEqual(changeList2.length, 3); + }); + + it('should throw on unknown verbs', async () => { + const fakeWatch = mock.mock(Watch); + const list: V1Namespace[] = []; + const listObj = { + metadata: { + resourceVersion: '12345', + } as V1ListMeta, + items: list, + } as V1NamespaceList; + const listFn: ListPromise = function (): Promise { + return new Promise((resolve) => { + resolve(listObj); + }); + }; + const informer = new ListWatch('/some/path', mock.instance(fakeWatch), listFn); + try { + informer.on('random' as any /* trick Typescript to allow this */, (obj) => {}); + fail('Unexpected lack of exception'); + } catch (err) { + deepEqual(err, Error('Unknown verb: random')); + } + try { + informer.off('random' as any /* trick Typescript to allow this */, (obj) => {}); + fail('Unexpected lack of exception'); + } catch (err) { + deepEqual(err, Error('Unknown verb: random')); + } + }); + + it('should handle off with callbacks that are not registered', async () => { + const fakeWatch = mock.mock(Watch); + const list: V1Namespace[] = []; + const listObj = { + metadata: { + resourceVersion: '12345', + } as V1ListMeta, + items: list, + } as V1NamespaceList; + const listFn: ListPromise = function (): Promise { + return new Promise((resolve) => { + resolve(listObj); + }); + }; + const informer = new ListWatch('/some/path', mock.instance(fakeWatch), listFn); + informer.off('add', (obj) => {}); + // No assertion because we're just looking to see if it throws. + }); + it('mutating handlers in a callback should not affect those which remain', async () => { const fakeWatch = mock.mock(Watch); const list: V1Namespace[] = []; From 85e1d2c30cdbdc6d47207779bcea139ae2423fda Mon Sep 17 00:00:00 2001 From: Brendan Burns <5751682+brendandburns@users.noreply.github.com> Date: Sat, 22 Feb 2025 23:23:36 +0000 Subject: [PATCH 2/2] Address comments --- src/cache_test.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/cache_test.ts b/src/cache_test.ts index f0142460f7..1d2438e3a2 100644 --- a/src/cache_test.ts +++ b/src/cache_test.ts @@ -1,4 +1,4 @@ -import { deepStrictEqual, deepEqual, fail, notStrictEqual, strictEqual, throws } from 'node:assert'; +import { deepStrictEqual, notStrictEqual, strictEqual, throws } from 'node:assert'; import mock from 'ts-mockito'; import { V1Namespace, V1NamespaceList, V1ObjectMeta, V1Pod, V1PodList, V1ListMeta } from './api.js'; @@ -887,18 +887,13 @@ describe('ListWatchCache', () => { }); }; const informer = new ListWatch('/some/path', mock.instance(fakeWatch), listFn); - try { + + throws(() => { informer.on('random' as any /* trick Typescript to allow this */, (obj) => {}); - fail('Unexpected lack of exception'); - } catch (err) { - deepEqual(err, Error('Unknown verb: random')); - } - try { + }, Error('Unknown verb: random')); + throws(() => { informer.off('random' as any /* trick Typescript to allow this */, (obj) => {}); - fail('Unexpected lack of exception'); - } catch (err) { - deepEqual(err, Error('Unknown verb: random')); - } + }, Error('Unknown verb: random')); }); it('should handle off with callbacks that are not registered', async () => {