Fix biases for "test-variations": ["Disabled", "Enabled"]

[nicolasanjoran]

Hi @rwbutler, Thanks for your work!

I think I found a small corner case when assigning "test-variations": ["Disabled", "Enabled"] instead of "test-variations": ["Enabled", "Disabled"]. When the order is reversed, biases aren't reversed. Not a big deal, but it could be annoying to release a feature to 90% of users, thinking only 10% would be impacted.

For example, with the following feature file:

{
    "features": [{
        "name": "Example Feature Flag",
        "enabled": true,
        "test-variations": ["Disabled", "Enabled"],
        "test-biases":[90, 10]
    }]
}

Calling Feature.named(.exampleFeatureFlag)!.description results in the following:

Feature: Example Feature Flag
Enabled: false
Test variations: Enabled (90.0%), Disabled (10.0%)
Test variation assignment: 93% -> Disabled

The same behaviour occurs with Off/On instead of Disabled/Enabled.

I made a quick and dirty patch for it, which preserves the method testVariationsContains in case you planned to use it for other use cases. It's not the most elegant way, but it seems to do the trick.

[rwbutler]

Hi @nicolasanjoran thanks for spotting this! 🙏

You're absolutely right this isn't what users would expect - appreciate you having a go at fixing this and was impressed by your fix working with so little code 👍

The only thing I thought was that looking back on this code in future, it might not be clear from the code what problem was to be solved by reversing the array of test biases. The problem is caused by the parsing code that existed before (not your fix) to be honest - when I wrote it a long time ago Swift was much newer (and I myself was newer to Swift).

Things have moved on significantly in Swift in the last five years and I saw a lot of opportunity to improve this parsing code to make it a lot clearer. Therefore I've refactored a lot of this parsing code here:

c247bb6

I think it now also solves the issue you experienced but please feel free to confirm and let me know if this is the case 🙏

Arguably, I could have gone a lot further to improve the rest of the parsing code not just parsing of test variations & biases but wanted to ensure that your use case was solved initially.

Hope this helps!

[rwbutler]

Added new unit tests to cover this use case: 1f993c9

[rwbutler]

@nicolasanjoran Have given you a credit in the CHANGELOG / release notes - hope this resolves the issue for you! 🙏

[nicolasanjoran]

Hey @rwbutler looks like it works now! Thanks a lot 😎