Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement equals for the ImmutableMetadata object #511

Closed
thomaspoignant opened this issue Jul 16, 2023 · 5 comments · Fixed by #512
Closed

Implement equals for the ImmutableMetadata object #511

thomaspoignant opened this issue Jul 16, 2023 · 5 comments · Fixed by #512
Assignees
Labels
bug Something isn't working

Comments

@thomaspoignant
Copy link
Member

Context

When working with an ImmutableMetadata it is hard to write tests because we cannot compare a FlagEvaluationDetails anymore.

String flagKey = "bool_targeting_match";
FlagEvaluationDetails expected = FlagEvaluationDetails.builder()
        .reason(Reason.ERROR.toString())
        .value(false)
        .errorCode(ErrorCode.GENERAL)
        .errorMessage("impossible to contact GO Feature Flag relay proxy instance")
        .flagMetadata(ImmutableMetadata.builder().build())
        .build();
FlagEvaluationDetails<Boolean> got = goffClient.getBooleanDetails(flagKey, false, defaultEvaluationContext);
assertEquals(expected, got);

The JUNIT method assertEquals is comparing the reference of the object.

Expected behavior

We should be able to use assertEquals for the whole FlagEvaluationDetails object.
For this we should probably implement the equals() method in the ImmutableMetadata object.

@justinabrahms
Copy link
Member

💯

@toddbaert
Copy link
Member

Should we consider other types that could benefit from this as well?. I don't think it's needed for EvaluationContext types, but I think it would be useful for Structure.

@thomaspoignant
Copy link
Member Author

Yes this is probably something we should look at.

I haven't seen other problem for now but I haven't test with Structure I will check it

@thomaspoignant
Copy link
Member Author

@toddbaert I've looked at Structure and it works already because we have the annotation in the ImmutableStructure.

I think we are good now, except if we want to introduce it for the EvaluationContext too.

@toddbaert
Copy link
Member

@toddbaert I've looked at Structure and it works already because we have the annotation in the ImmutableStructure.

I think we are good now, except if we want to introduce it for the EvaluationContext too.

Thanks @thomaspoignant !

I could see some use-cases for hasing/comparing EvaluationContext, but I think they are limited. The one that comes to mind immediately is for comparing it in a provider, for some kind of caching, maybe. I'll think about it more but we can always address it in a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants