Unleash / unleash-client-go

Unleash client SDK for Go
https://docs.getunleash.io
Apache License 2.0
138 stars 55 forks source link

Revert "feat: add FeatureEnabled to Variant" #165

Closed FredrikOseberg closed 10 months ago

FredrikOseberg commented 10 months ago

Reverts Unleash/unleash-client-go#164

Looks like the changes caused the spec tests to fail: https://github.com/Unleash/unleash-client-go/actions/runs/7178688374/job/19547365015

This is surprising because the workflow should run the spec tests when you setup a new PR, but for some reason this time they weren't triggered.

CC: @jameshartig

Could you check if the spec tests pass locally with your changes? There should be instructions here. If not I can try to do that later.

jameshartig commented 10 months ago

@FredrikOseberg Sorry about that! I didn't realize I needed to check out the client specs for those tests to run. I'll work on that now real quick.

jameshartig commented 10 months ago

@FredrikOseberg I admittedly don't have a lot of experience with these tests but It looks like my code is actually working but interpreting the spec is broken.

=== RUN   TestClientSpecificationSuite/TestClientSpecification/17-dependent-features/Child_yields_its_variant_when_both_parent_and_child_are_enabled.
    spec_test.go:91:
                Error Trace:    spec_test.go:91
                Error:          Not equal:
                                expected: &api.Variant{Name:"child.variant", Payload:api.Payload{Type:"string", Value:"variantValue"}, Enabled:true, FeatureEnabled:false}
                                actual  : &api.Variant{Name:"child.variant", Payload:api.Payload{Type:"string", Value:"variantValue"}, Enabled:true, FeatureEnabled:true}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -7,3 +7,3 @@
                                  Enabled: (bool) true,
                                - FeatureEnabled: (bool) false
                                + FeatureEnabled: (bool) true
                                 })
                Test:           TestClientSpecificationSuite/TestClientSpecification/17-dependent-features/Child_yields_its_variant_when_both_parent_and_child_are_enabled.

But the spec for that says:

    {
      "description": "Child yields its variant when both parent and child are enabled.",
      "context": {},
      "toggleName": "parent.enabled.child.enabled",
      "expectedResult": {
        "name": "child.variant",
        "payload": {
          "type": "string",
          "value": "variantValue"
        },
        "enabled": true,
        "feature_enabled": true
      }
    }

I wonder if the issue is that feature_enabled is using underscores the api.Variant's FeatureEnabled field is featureEnabled.