launchdarkly / openfeature-node-server

An open feature provider for the LaunchDarkly node SDK.
Other
10 stars 3 forks source link

fix: unit test failures #23

Closed yusinto closed 1 year ago

yusinto commented 1 year ago

It looks like gonfalon may have updated the eval response and nown all unit tests are failing with two additional fields flagMetadata and variant. I see that variant is part of the open feature spec, but flagMetadata is not. This pr takes the easy way out and changes the assertion to match a subset of the json response, but I'm not sure if this is the original intent of these tests.

 expect(received).toEqual(expected) // deep equality

    - Expected  - 0
    + Received  + 2

      Object {
        "flagKey": "a-key",
    +   "flagMetadata": Object {},
        "reason": "OFF",
        "value": Object {
          "some": "value",
        },
    +   "variant": undefined,
      }
shortcut-integration[bot] commented 1 year ago

This pull request has been linked to Shortcut Story #207882: Fix openfeature node-server failing unit tests.

kinyoklion commented 1 year ago

@yusinto These do not use gonfalon.

    ldClient.variationDetail = jest.fn(async () => ({
      value: true,
      reason: {
        kind: 'OFF',
      },
    }));

No LD client is made, instead we are just passing specific return values for the methods.

The issue with the test here is the result of an upgrade to the OpenFeature SDK. It now returns extra fields that have been added to the OpenFeature spec.

https://github.com/open-feature/spec/blob/2c9344c443bbea1271e03d0855d63d1d9ba8d682/specification/sections/01-flag-evaluation.md?plain=1#L252C3-L252C3

I also have these changes in the 8.0 upgrade PR, which should be merged this week.

As we are not populating additional information for those fields, I do think it is fine to just ignore them, as both our changes do, for now.

kinyoklion commented 1 year ago

Closing this because the changes were in the V8 upgrade.