Skip to content

chore: caching.feature integration tests #249

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

chore: caching.feature integration tests #249

wants to merge 4 commits into from

Conversation

@skyerus
Copy link
Contributor

@skyerus skyerus commented Jan 17, 2023

This PR

Implements caching test suite.

Related Issues

Fixes #243

Notes

Follow-up Tasks

How to test

@skyerus skyerus marked this pull request as draft January 17, 2023 14:21
@skyerus
Copy link
Contributor Author

skyerus commented Jan 17, 2023 •
edited
Loading

Converted to draft until v0.5.5 of flagd-provider is released

@skyerus skyerus mentioned this pull request Jan 17, 2023
Merged
@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from d850d0c to 04b94fd Compare January 18, 2023 09:24
@skyerus
Copy link
Contributor Author

skyerus commented Jan 18, 2023 •
edited
Loading

Running into some inconsistent test failures here, the flakey results of tests are implying that the flagd provider is either exhibiting unexpected asynchronous behaviour or there are event stream connection issues.
This is one of the tests that is passing/failing intermittently:

Given a boolean flag with key "boolean-flag" is evaluated with details and default value "false"
When a boolean flag with key "boolean-flag" is evaluated with details and default value "false"
Then the resolved boolean details reason should be "CACHED"

Stepping through the code, my understanding is that a boolean evaluation stores the value in cache synchronously in the first call then when the second call is invoked the value is retrieved from the cache. Either my understanding about the synchronous nature is wrong or there is some kind of disconnection happening with the event stream that is causing the cache to be ignored (by design).

Will debug further.

@skyerus
Copy link
Contributor Author

skyerus commented Jan 18, 2023

This exposure of a blocking mechanism until the event stream is alive fixes the flakiness: open-feature/java-sdk-contrib#204

@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from c976ef5 to 7ba569d Compare January 19, 2023 09:12
@skyerus skyerus mentioned this pull request Jan 19, 2023
Merged
@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from dc682be to d7e185f Compare January 19, 2023 14:56
@skyerus skyerus marked this pull request as ready for review January 19, 2023 15:55
toddbaert
toddbaert previously approved these changes Jan 19, 2023
View reviewed changes
@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch 2 times, most recently from 571d7ef to 18ed236 Compare January 20, 2023 16:11
toddbaert
toddbaert previously approved these changes Jan 20, 2023
View reviewed changes
@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from 18ed236 to abe045b Compare January 20, 2023 16:14
Kavindu-Dodan
Kavindu-Dodan previously approved these changes Jan 23, 2023
View reviewed changes
@toddbaert
Copy link
Member

@skyerus The caching tests here appear to be hanging. I've re-run them a few times.

@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from 3352582 to 9d952dc Compare January 27, 2023 12:55
@skyerus skyerus marked this pull request as draft January 27, 2023 14:20
@skyerus
chore: caching.feature integration tests
07266d1 
Signed-off-by: Skye Gill <gill.skye95@gmail.com>
@skyerus
mount volume in ci test
b531a3d 
Signed-off-by: Skye Gill <gill.skye95@gmail.com>
@skyerus
block until event stream is alive
22f8893 
Signed-off-by: Skye Gill <gill.skye95@gmail.com>
@skyerus
improved logging
946bca1 
Signed-off-by: Skye Gill <gill.skye95@gmail.com>
@skyerus skyerus force-pushed the issue-243_caching-integration-tests branch from 32a1933 to 946bca1 Compare January 30, 2023 13:49
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@toddbaert toddbaert self-assigned this Jan 30, 2023
@toddbaert
Copy link
Member

I'll look into what's going on here.

@justinabrahms
Copy link
Member

@toddbaert Is this still something to look into? Or close?

@beeme1mr
Copy link
Member

beeme1mr commented May 4, 2023

This PR appears to be stale. Please reopen if you would like to continue working on it.

@beeme1mr beeme1mr closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan left review comments

@toddbaert toddbaert toddbaert left review comments

@justinabrahms justinabrahms Awaiting requested review from justinabrahms

@agentgonzo agentgonzo Awaiting requested review from agentgonzo

Assignees

@toddbaert toddbaert

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement caching integration tests

5 participants

@skyerus @toddbaert @justinabrahms @beeme1mr @Kavindu-Dodan