-
Notifications
You must be signed in to change notification settings - Fork 17
feat: send user agent header #77
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
5cea695
b184964
e7b671f
97643e4
6416cb7
2d6527f
c1c4edc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to somehow mock the version so it can be seen in at least one test? All current tests only verify
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @emyller I've tried both PowerMock and MockK and there doesn't seem to be a way that I can find that can sufficiently mock the Class.forName() functionality. I could refactor getSdkVersion into an interface so that I can mock the dependency into the main Retrofit class but again it doesn't really test any extra code and adds more complexity. I'm a lot happier with the code now that we're falling back to release-please as per the iOS SDK and the updated function looks pretty bullet proof and as tested as I can get it really.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check out the updated implementation and tests @emyller and let me know what you think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the fate of this thread will lie on this other thread. On the good news, I believe this becomes much simpler. See example. Let me know if you have any questions.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be sorted now @emyller please check, thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @gazreese. Sorry I didn't make it clear in my comment above. What I really meant by "much simpler" is that we may not need these tests at all. From my perspective, As pointed out in the other thread, it may even suffice as the only test in this PR. The others feel more of the same, but I'd love to hear your thoughts here. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| package com.flagsmith | ||
|
|
||
| import com.flagsmith.entities.Trait | ||
| import com.flagsmith.mockResponses.MockEndpoint | ||
| import com.flagsmith.mockResponses.mockResponseFor | ||
| import kotlinx.coroutines.runBlocking | ||
| import org.junit.After | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Before | ||
| import org.junit.Test | ||
| import org.mockserver.integration.ClientAndServer | ||
| import org.mockserver.model.HttpRequest.request | ||
|
|
||
| class UserAgentTests { | ||
|
|
||
| private lateinit var mockServer: ClientAndServer | ||
| private lateinit var flagsmith: Flagsmith | ||
|
|
||
| @Before | ||
| fun setup() { | ||
| mockServer = ClientAndServer.startClientAndServer() | ||
| } | ||
|
|
||
| @After | ||
| fun tearDown() { | ||
| mockServer.stop() | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithValidVersion() { | ||
| // Given - The User-Agent now shows SDK version or "unknown" (not app version) | ||
| // This is because getUserAgent() method was updated to return SDK version | ||
| // In tests, BuildConfig is not available, so it returns "unknown" | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.GET_FLAGS) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.getFeatureFlagsSync() | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then - Verify User-Agent contains "unknown" since BuildConfig is not available in tests | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/flags/") | ||
| .withMethod("GET") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithNullContext() { | ||
| // Given | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.GET_FLAGS) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.getFeatureFlagsSync() | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/flags/") | ||
| .withMethod("GET") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithExceptionDuringVersionRetrieval() { | ||
| // Given - Even with context, getUserAgent() now returns SDK version or "unknown" | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.GET_FLAGS) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.getFeatureFlagsSync() | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/flags/") | ||
| .withMethod("GET") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithNullVersionName() { | ||
| // Given - getUserAgent() now returns SDK version or "unknown" regardless of context | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.GET_FLAGS) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.getFeatureFlagsSync() | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/flags/") | ||
| .withMethod("GET") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithIdentityRequest() { | ||
| // Given - getUserAgent() now returns SDK version or "unknown" | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.GET_IDENTITIES) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.getIdentitySync("test-user") | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then - Verify User-Agent contains "unknown" since BuildConfig is not available in tests | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/identities/") | ||
| .withMethod("GET") | ||
| .withQueryStringParameter("identifier", "test-user") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun testUserAgentHeaderSentWithTraitRequest() { | ||
| // Given - getUserAgent() now returns SDK version or "unknown" | ||
| flagsmith = Flagsmith( | ||
| environmentKey = "test-key", | ||
| baseUrl = "http://localhost:${mockServer.localPort}", | ||
| context = null, | ||
| enableAnalytics = false, | ||
| cacheConfig = FlagsmithCacheConfig(enableCache = false) | ||
| ) | ||
|
|
||
| mockServer.mockResponseFor(MockEndpoint.SET_TRAIT) | ||
|
|
||
| // When | ||
| runBlocking { | ||
| val result = flagsmith.setTraitSync(Trait(key = "test-key", traitValue = "test-value"), "test-user") | ||
| assertTrue(result.isSuccess) | ||
| } | ||
|
|
||
| // Then - Verify the traits request has correct User-Agent with "unknown" since BuildConfig is not available in tests | ||
| mockServer.verify( | ||
| request() | ||
| .withPath("/identities/") | ||
| .withMethod("POST") | ||
| .withHeader("User-Agent", "flagsmith-kotlin-android-sdk/unknown") | ||
| ) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.