fix: Include impression data status when not enabled (#122)

## What

This PR fixes a bug in how impression data is reported when `impressionDataAll` is set to true: When a feature has `impressionData: false` but `impressionDataAll` is set to true, it will now report `impressionData: false` instead of `impressionData: undefined`.

## Why

This is a bug because we should report the state of impression data for a feature when we can. In the original feature PR discussion, it was agreed to make `impressionData` a `boolean | undefined` where it would be reported as a boolean if we knew the state and `undefined` otherwise.

It seems to have been an oversight in the tests.

## Clarifications

### What's with the try/catch blocks in the tests?

It's explained more thoroughly in the [callback section of Jest's testing asynchronous code](https://jestjs.io/docs/asynchronous#callbacks), but in short:

If you don't have the try/catch and one of the expectations fail, then the done callback is never called (because the function short circuits). While this does mean that the test fails (this is good), it fails with a message saying that it exceeded the set timeout and not because it failed an expectation (this is bad).

To instead have the correct error message reported, you must catch the error and pass it to the done callback. This will make the test fail and report the correct error message.

Now regarding using `finally`, I did consider that but ended up not doing it for the following reason:

Calling the done callback requires the error for it to fail. This can't be done in a finally block because you don't have access to the error. In theory, you could probably declare an error variable that you set to undefined and then assign in the catch block, but that doesn't feel any cleaner to me.

## Commits

* fix: tests expect false impression data status to be included

* fix: fix tests to use try/catch as described in the Jest docs

Refer to: https://jestjs.io/docs/asynchronous#callbacks

* fix: update how the impressionData property gets set

Set it to what's defined on the toggle if anything is. Otherwise, use undefined.

* refactor: rename tests to better describe what they check for

I had trouble understanding what the different tests were meant to
check for, so I've renamed them into something (hopefully) more explanatory.

src/index.test.ts

@@ -1023,7 +1023,7 @@ test('Should pass under custom header clientKey', async () => {
     jest.advanceTimersByTime(999);
 });
 
-test('Should call isEnabled event when impressionData is true', (done) => {
+test('Should emit impression events on isEnabled calls when impressionData is true', (done) => {
     const bootstrap = [
         {
             name: 'impression',
@@ -1084,7 +1084,7 @@ test('Should pass custom headers', async () => {
     });
 });
 
-test('Should call getVariant event when impressionData is true', (done) => {
+test('Should emit impression events on getVariant calls when impressionData is true', (done) => {
     const bootstrap = [
         {
             name: 'impression-variant',
@@ -1120,7 +1120,7 @@ test('Should call getVariant event when impressionData is true', (done) => {
     });
 });
 
-test('Should not call isEnabled event when impressionData is false', (done) => {
+test('Should not emit impression events on isEnabled calls when impressionData is false', (done) => {
     const bootstrap = [
         {
             name: 'impression',
@@ -1155,7 +1155,7 @@ test('Should not call isEnabled event when impressionData is false', (done) => {
     });
 });
 
-test('Should call isEnabled event when impressionData is false and impressionDataAll is true', (done) => {
+test('Should emit impression events on isEnabled calls when impressionData is false and impressionDataAll is true', (done) => {
     const bootstrap = [
         {
             name: 'impression',
@@ -1184,15 +1184,20 @@ test('Should call isEnabled event when impressionData is false and impressionDat
     });
 
     client.on(EVENTS.IMPRESSION, (event: any) => {
-        expect(event.featureName).toBe('impression');
-        expect(event.eventType).toBe('isEnabled');
-        expect(event.impressionData).toBe(undefined);
-        client.stop();
-        done();
+        try {
+            expect(event.featureName).toBe('impression');
+            expect(event.eventType).toBe('isEnabled');
+            expect(event.impressionData).toBe(false);
+            client.stop();
+            done();
+        } catch (e) {
+            client.stop();
+            done(e);
+        }
     });
 });
 
-test('Should call isEnabled event when toggle is unknown and impressionDataAll is true', (done) => {
+test('Should emit impression events on isEnabled calls when toggle is unknown and impressionDataAll is true', (done) => {
     const bootstrap = [
         {
             name: 'impression',
@@ -1230,7 +1235,7 @@ test('Should call isEnabled event when toggle is unknown and impressionDataAll i
     });
 });
 
-test('Should call getVariant event when impressionData is false and impressionDataAll is true', (done) => {
+test('Should emit impression events on getVariant calls when impressionData is false and impressionDataAll is true', (done) => {
     const bootstrap = [
         {
             name: 'impression-variant',
@@ -1259,11 +1264,16 @@ test('Should call getVariant event when impressionData is false and impressionDa
     });
 
     client.on(EVENTS.IMPRESSION, (event: any) => {
-        expect(event.featureName).toBe('impression-variant');
-        expect(event.eventType).toBe('getVariant');
-        expect(event.impressionData).toBe(undefined);
-        client.stop();
-        done();
+        try {
+            expect(event.featureName).toBe('impression-variant');
+            expect(event.eventType).toBe('getVariant');
+            expect(event.impressionData).toBe(false);
+            client.stop();
+            done();
+        } catch (e) {
+            client.stop();
+            done(e);
+        }
     });
 });
 

src/index.ts

@@ -194,7 +194,7 @@ export class UnleashClient extends TinyEmitter {
                 enabled,
                 toggleName,
                 IMPRESSION_EVENTS.IS_ENABLED,
-                toggle?.impressionData || undefined,
+                toggle?.impressionData ?? undefined,
             );
             this.emit(EVENTS.IMPRESSION, event);
         }
@@ -214,7 +214,7 @@ export class UnleashClient extends TinyEmitter {
                 enabled,
                 toggleName,
                 IMPRESSION_EVENTS.GET_VARIANT,
-                toggle?.impressionData || undefined,
+                toggle?.impressionData ?? undefined,
                 variant.name,
             );
             this.emit(EVENTS.IMPRESSION, event);