Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Include impression data status when not enabled #122

Merged
merged 4 commits into from
Oct 31, 2022
Merged

fix: Include impression data status when not enabled #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78b7ed1
fix: tests expect false impression data status to be included
thomasheartman Oct 26, 2022
c674561
fix: fix tests to use try/catch as described in the Jest docs
thomasheartman Oct 26, 2022
be0c8f8
fix: update how the impressionData property gets set
thomasheartman Oct 26, 2022
3c0ff8a
refactor: rename tests to better describe what they check for
thomasheartman Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 26 additions & 16 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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 {
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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);
}
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down