fix: Include impression data status when not enabled

[thomasheartman]

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, 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.

[sighphyre]

This is such a subtle change that I think that if it was worth fixing then it's worth adding a test for. I agree with the source code changes but the new try/catches look odd to me

[sighphyre]

Nice