fix: Update tests (#147)

This PR does three things:

- Adds tests for undefined/null properties 
- Updates test titles
- Formats the test suites

The new tests were brought on by #146

## New tests

I've added two (three) new tests.
1. `src/util.test.ts`: Verify that object properties set to `null` or `undefined` are excluded from query params
2. `src/index.test.ts`: Verify that setting a context field to `null` or `undefined` removes it from the context

## Test title updates

Two of the tests in `src/util.test.ts` seems to have had copy-paste mistakes in their titles. Looking at them, I believe the "not" in their descriptions should be removed.

## Formatting

This repo has prettier listed in its deps, but the test files were pretty unformatted. My editor auto-formats on save, so this led to a lot of changes. However, I've isolated the formatting changes into separate commits, so we can easily cut those out if we don't want them.

Diffing with **hidden whitespace** is encouraged.

## Commits

* fix: reword test names + change ts-ignore to ts-expect-error

Looking at the tests, I can only assume that the "not" in their names
was a copy-paste error.

* format: format test suite

* fix: add new test that checks null and undefined properties

* fix: add test that clearing context works as expected

* format: run prettier on tests

* Fix: update ts-expect-error comment

* add check for wrapped strings of null or undefined when filtering params

* revert

---------

Co-authored-by: andreas-unleash <andreas@getunleash.ai>

src/index.test.ts

@@ -71,7 +71,6 @@ test('Should perform an initial fetch as GET', async () => {
     expect(request.method).toBe('GET');
 });
 
-
 test('Should have correct variant', async () => {
     fetchMock.mockResponseOnce(JSON.stringify(data));
     const config: IConfig = {
@@ -564,23 +563,25 @@ test('Should publish error when fetch fails', (done) => {
     });
 });
 
-test.each([400, 401, 403, 404, 429, 500, 502, 503])('Should publish error when fetch receives a %d error', async (errorCode) => {
-    expect.assertions(1);
-    jest.spyOn(global.console, 'error').mockImplementation(() => jest.fn());
-    fetchMock.mockResponseOnce("{}", { status: errorCode});
+test.each([400, 401, 403, 404, 429, 500, 502, 503])(
+    'Should publish error when fetch receives a %d error',
+    async (errorCode) => {
+        expect.assertions(1);
+        jest.spyOn(global.console, 'error').mockImplementation(() => jest.fn());
+        fetchMock.mockResponseOnce('{}', { status: errorCode });
 
-    const config: IConfig = {
-        url: 'http://localhost/test',
-        clientKey: '12',
-        appName: 'web',
-    };
-    const client = new UnleashClient(config);
-    client.on(EVENTS.ERROR, (e: any) => {
-        expect(e).toStrictEqual({ type: 'HttpError', code: errorCode});
-
-    });
-    await client.start();
-})
+        const config: IConfig = {
+            url: 'http://localhost/test',
+            clientKey: '12',
+            appName: 'web',
+        };
+        const client = new UnleashClient(config);
+        client.on(EVENTS.ERROR, (e: any) => {
+            expect(e).toStrictEqual({ type: 'HttpError', code: errorCode });
+        });
+        await client.start();
+    }
+);
 
 test('Should publish update when state changes after refreshInterval', async () => {
     expect.assertions(1);
@@ -852,7 +853,6 @@ test('Should update context fields with await', async () => {
     expect(url.searchParams.get('environment')).toEqual('prod');
 });
 
-
 test('Should update context fields on request', async () => {
     fetchMock.mockResponses(
         [JSON.stringify(data), { status: 200 }],
@@ -905,7 +905,7 @@ test('Updating context should wait on asynchronous start', async () => {
 
     client.start();
     await client.updateContext({
-        userId: '123'
+        userId: '123',
     });
 
     expect(fetchMock).toBeCalledTimes(2);
@@ -938,7 +938,9 @@ test('Should not replace sessionId when updating context', async () => {
 
     const url = new URL(getTypeSafeRequestUrl(fetchMock));
 
-    expect(url.searchParams.get('sessionId')).toEqual(context.sessionId?.toString());
+    expect(url.searchParams.get('sessionId')).toEqual(
+        context.sessionId?.toString()
+    );
 });
 
 test('Should not add property fields when properties is an empty object', async () => {
@@ -1423,3 +1425,25 @@ test("Should update toggles even when refresh interval is set to '0'", async ()
     await client.updateContext({ userId: '123' });
     expect(fetchMock).toHaveBeenCalledTimes(2);
 });
+
+test.each([null, undefined])(
+    'Setting a context field to %s should clear it from the context',
+    async () => {
+        fetchMock.mockResponse(JSON.stringify(data));
+        const config: IConfig = {
+            url: 'http://localhost/test',
+            clientKey: '12',
+            appName: 'web',
+        };
+        const client = new UnleashClient(config);
+        await client.start();
+
+        await client.updateContext({ userId: '123' });
+        expect(client.getContext().userId).toEqual('123');
+
+        const userId = undefined;
+        await client.updateContext({ userId });
+
+        expect(client.getContext().userId).toBeUndefined();
+    }
+);

src/util.test.ts

@@ -1,28 +1,60 @@
-import {urlWithContextAsQuery} from './util';
+import { urlWithContextAsQuery } from './util';
 
 test('should not add paramters to URL', async () => {
-    const someUrl = new URL("https://test.com");
+    const someUrl = new URL('https://test.com');
 
-    //@ts-ignore on purpose for testing!
+    //@ts-expect-error on purpose for testing!
     const result = urlWithContextAsQuery(someUrl, {});
 
     expect(result.toString()).toBe('https://test.com/');
 });
 
+test('should add context as query params', async () => {
+    const someUrl = new URL('https://test.com');
 
-test('should not add context as query params', async () => {
-    const someUrl = new URL("https://test.com");
+    const result = urlWithContextAsQuery(someUrl, {
+        appName: 'test',
+        userId: '1234A',
+    });
 
-    const result = urlWithContextAsQuery(someUrl, {appName: 'test', userId: '1234A'});
-
-    expect(result.toString()).toBe('https://test.com/?appName=test&userId=1234A');
+    expect(result.toString()).toBe(
+        'https://test.com/?appName=test&userId=1234A'
+    );
 });
 
+test('should add context properties as query params', async () => {
+    const someUrl = new URL('https://test.com');
+
+    const result = urlWithContextAsQuery(someUrl, {
+        appName: 'test',
+        userId: '1234A',
+        properties: { custom1: 'test', custom2: 'test2' },
+    });
 
-test('should not add context properties as query params', async () => {
-    const someUrl = new URL("https://test.com");
+    expect(result.toString()).toBe(
+        'https://test.com/?appName=test&userId=1234A&properties%5Bcustom1%5D=test&properties%5Bcustom2%5D=test2'
+    );
+});
 
-    const result = urlWithContextAsQuery(someUrl, {appName: 'test', userId: '1234A', properties: { custom1: 'test', custom2: "test2"}});
+test('should exclude context properties that are null or undefined', async () => {
+    const someUrl = new URL('https://test.com');
+
+    const result = urlWithContextAsQuery(someUrl, {
+        appName: 'test',
+        userId: undefined,
+        properties: {
+            custom1: 'test',
+            custom2: 'test2',
+            //@ts-expect-error this shouldn't be allowed if you're using TS, but
+            //you could probably force it
+            custom3: null,
+            //@ts-expect-error same as the null property above
+            custom4: undefined,
+        },
+    });
+
+    expect(result.toString()).toBe(
+        'https://test.com/?appName=test&properties%5Bcustom1%5D=test&properties%5Bcustom2%5D=test2'
+    );
+});
 
-    expect(result.toString()).toBe('https://test.com/?appName=test&userId=1234A&properties%5Bcustom1%5D=test&properties%5Bcustom2%5D=test2');
-});

src/util.ts

@@ -29,4 +29,4 @@ export const urlWithContextAsQuery = (url: URL, context: IContext) => {
             }
         });
     return urlWithQuery;
-}
+}