feat: Update test harness with metadata assertions #1467

[chrfwow]

This PR

Adds gherkin test to verify that flag evaluations provide metadata

Related Issues

Part of #1467 (open-feature/flagd#1467)

Follow-up Tasks

Implement steps of the gherkin file in the repositories, and add test data according to the issue

[toddbaert]

Suggested change

	Given a Boolean-flag with key "metadata-flag" and a default value "true"
	When the flag was evaluated with details
	Given a Boolean-flag with key "metadata-flag" is evaluated with default value "false"

Let's use the same sentence structure as the other steps, this way the same bindings will "just work" and less code needs to be implemented.

[aepfli]

I would specifically separate those two. Sometimes, you need to flag information without an evaluation. Hence, it is only a given state. But the when is the dedicated action. eg when it was modified. With this, we separate data and action. We already did the same thing within the testbed.

[toddbaert]

I'm not sure these empty-metadata tests are worth it TBH. cc @aepfli what do you think?

[aepfli]

yeah this was more a little exploration showcasing from my side, i still think that a negative case, to ensure we are not polluting the response makes sense

[toddbaert]

We could probably use a table for this, with columns for the metadata key, type, and value.

[toddbaert]

@aepfli do you think it makes more sense to simply add metadata to the existing gherkin suite? We already have error flags there - we could add metadata to some of the existing flags and error flags, and them update the in-memory providers to support this.

[aepfli]

@aepfli do you think it makes more sense to simply add metadata to the existing gherkin suite? We already have error flags there - we could add metadata to some of the existing flags and error flags, and them update the in-memory providers to support this.

I am not sure if integration into the existing suite is suitable. We could enhance the existing ones, with a description of what the purpose of those flags is. But generally i prefer a flagname which already tells me what i can expect from this flag.

This also brings me to the next topic. As we are currently generating the data in each sdk on our own, it might be worth to add a json here with the testdata, which we load into the inmemory provider, wdt?

[guidobrei]

Metadata type should be Float

[toddbaert]

This also brings me to the next topic. As we are currently generating the data in each sdk on our own, it might be worth to add a json here with the testdata, which we load into the inmemory provider, wdt?

Most in-memory providers can't actually load JSON, so I'm not sure how valuable that would be.

[aepfli]

This also brings me to the next topic. As we are currently generating the data in each sdk on our own, it might be worth to add a json here with the testdata, which we load into the inmemory provider, wdt?

Most in-memory providers can't actually load JSON, so I'm not sure how valuable that would be.

i would not load them automagically, but within the tests, we can load the json files, and provide the data as parameter to the inmemory provider. We would centralize the test flag data, and with new gherkin files, which only add new data for experimentation everything should work ootb.

data = loadJsonData(jsonfile)
provider = InMemoryProvider(data) // simple prosa

[toddbaert]

This also brings me to the next topic. As we are currently generating the data in each sdk on our own, it might be worth to add a json here with the testdata, which we load into the inmemory provider, wdt?

Most in-memory providers can't actually load JSON, so I'm not sure how valuable that would be.

i would not load them automagically, but within the tests, we can load the json files, and provide the data as parameter to the inmemory provider. We would centralize the test flag data, and with new gherkin files, which only add new data for experimentation everything should work ootb.

data = loadJsonData(jsonfile)
provider = InMemoryProvider(data) // simple prosa

Oh, so all implementations would write a simple function loadJsonData or similar which would do the conversion? I guess that's not a bad idea since it might be fairly future-proof when new things are added.

[aepfli]

Oh, so all implementations would write a simple function loadJsonData or similar which would do the conversion? I guess that's not a bad idea since it might be fairly future-proof when new things are added.

excactly, and this way we ensure, everyone gets the same data afterall