coldbox-modules / LaunchDarklySDK

A CFML SDK for Launch Darkly feature flags
8 stars 4 forks source link

Updating Java SDK to v6.0.6 (Failing tests related to Change Listeners) #4

Closed SMSMichael closed 1 year ago

SMSMichael commented 1 year ago

Description

This PR is an attempt to create a minimum viable product (MVP) update that updates the underlying LaunchDarkly Java SDK to the latest version 6 (6.0.6) (see #3). As an MVP update, I am not attempting to introduce support for the new Context targeting system introduced by v6, but instead attempting to get current implementations based on the v5 working in v6.

Changes Introduced

I introduced the following changes:

  1. I updated the test-flags.json file to include a "string-feature-with-targeting" example that includes a populated targets attribute, as well as includes the new contextTargets attribute (purpose: testing new v6 syntax changes)
  2. I added a new testbox test to LDTest.cfc to validate that flag targeting was still working as expected ("can correctly target a flag variation based on the user") (purpose: testing new v6 syntax changes)
  3. I introduced a change to the test suite file organization as a way to group the tests by category so I could better target which tests are run as a group. Please note that this change was made as a personal preference and was done without consideration of what performance impact (if any) might result from breaking the suite up into multiple describe() blocks, or any existing expectations around testing style guides. Please feel free to revert this change to return to the format the tests were originally in.

Issues Resolved & Remaining

When the SDK version was swapped out in this branch, I encountered 3 failing unit tests:

 No matching Method/Function for com.launchdarkly.sdk.server.FlagTrackerImpl.addFlagChangeListener(V5f2f98d9753dd9c1710c6f46fb57adb17231) found

and

No matching Method/Function for com.launchdarkly.sdk.server.FlagTrackerImpl.addFlagValueChangeListener(string, com.launchdarkly.sdk.LDUser, V893214323706b07d5121c1e623a9db517687) found

Hoping someone else with a deeper understanding of the underlying Java proxy system can spot the issue.

bdw429s commented 1 year ago

Hi @SMSMichael, I've been super busy but I'm finally getting a chance to look at this. I'm guessing you didn't any further with the remaining issue. Unless you have any more code to add here, I'll probably merge this and continue working on it.

bdw429s commented 1 year ago

@SMSMichael I pulled your changes and good news-- they are 100% passing on both Lucee 5.3.10 and Adobe CF 2023. You never said what CF engine/version you were getting the errors on, but perhaps it was a CF engine bug?