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

feat: isolate interfaces from SDK to improve testability #268

Merged
merged 7 commits into from
May 29, 2024

Conversation

Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Apr 29, 2024

This PR

Partially address #266 by providing interface contracts for the SDK. There are no breaking changes in this change

I have introduced IEvaluation interface, which represents the general OpenFeatuer API layer [1]. Before this change, there was no such interface so mocking API interactions was impossible. To obtain the singleton instance of IEvaluation interface, I have introduced GetApiInstance() helper.

Consider the example below,

// get the global instance that may be injected into testable components 
apiInstance := openfeature.GetApiInstance()

// use for OpenFeature operations 
apiInstance.SetProvider(myProvider)
apiInstance.AddHandler(....)

In the future, we can update our documentation to make API instance based operations the preferred interaction method.

Further, IEvaluation exposes client creation methods GetClient() & GetNamedClient(clientName string) (similar to spec [2]). These methods return IClient instances, making it easy to test OpenFeature client related operations.

apiInstance := openfeature.GetApiInstance()

// IClient instance, which can be mocked when injected into preferred locations 
client := apiInstance.GetClient()

[1] - https://openfeature.dev/specification/sections/flag-evaluation
[2] - https://openfeature.dev/specification/sections/flag-evaluation#creating-clients

@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch from 0e316a3 to 10e29be Compare April 29, 2024 22:43
Copy link

codecov bot commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 81.92%. Comparing base (aeb4f6c) to head (c7ede79).

Files Patch % Lines
openfeature/client.go 72.22% 5 Missing ⚠️
openfeature/openfeature_api.go 93.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #268      +/-   ##
==========================================
+ Coverage   81.74%   81.92%   +0.17%     
==========================================
  Files          10       10              
  Lines         964      979      +15     
==========================================
+ Hits          788      802      +14     
  Misses        156      156              
- Partials       20       21       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch from efeff46 to 57db4d6 Compare May 1, 2024 16:18
@Kavindu-Dodan Kavindu-Dodan changed the title feat: [WIP] isolate interfaces from SDK & derive test utilities feat: [WIP] isolate interfaces from SDK to improve testability May 1, 2024
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch from 57db4d6 to 24041fa Compare May 1, 2024 16:19
@Kavindu-Dodan Kavindu-Dodan changed the title feat: [WIP] isolate interfaces from SDK to improve testability feat: isolate interfaces from SDK to improve testability May 1, 2024
@Kavindu-Dodan Kavindu-Dodan marked this pull request as ready for review May 1, 2024 16:25
@Kavindu-Dodan Kavindu-Dodan requested a review from a team as a code owner May 1, 2024 16:25
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch 2 times, most recently from a6c2c55 to 1af6cf0 Compare May 1, 2024 16:50
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch 3 times, most recently from 76d0a50 to 275a505 Compare May 8, 2024 17:16
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch from 64f0598 to 30e3e4e Compare May 16, 2024 21:28
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/improve-testability branch from 30e3e4e to c7ede79 Compare May 23, 2024 17:10
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, and we've made similar choices regarding the interfaces in other implementations. I'm approving but I think others with more Go experience than me should also have their input (thanks to @hairyhenderson for his feedback so far 🙏 )

@Kavindu-Dodan Kavindu-Dodan added this to the 1.12.0 milestone May 28, 2024
@toddbaert toddbaert merged commit 5e06c45 into open-feature:main May 29, 2024
6 checks passed
@hairyhenderson
Copy link
Contributor

sorry for the slow response on this, and thanks for merging it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants