Add OkHttpClientProvider tests#48958
Conversation
cortinico
left a comment
There was a problem hiding this comment.
Also not a huge fan of us using reflection here. But if you plan on migrating OkHttpClientProvider to Kotlin, then you can make those fields internal to make this test cleaner
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Co-authored-by: Nicola Corti <corti.nico@gmail.com>
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Thanks for the feedback @cortinico, I cleaned this up a bit in 36b5556. Will make sure to clean up the reflection as well when migrating the class to Kotlin |
Never mind. I actually have to commit a different version of this file, due to differences in the OkHTTP version between internal and OSS. I'd just skip 36b5556 if you don't mind and we can follow-up on the cleanup after your migration |
|
@cortinico merged this pull request in 9572bcf. |
|
This pull request was successfully merged by @mateoguzmana in 9572bcf When will my fix make it into a release? | How to file a pick request? |
Summary: Migrate com.facebook.react.modules.network.OkHttpClientProvider to Kotlin. Also, as follow up from #48958 I'm cleaning up the reflection on `OkHttpClientProviderTest` as we can make `sClient` and `sFactory` internal. ## Changelog: [INTERNAL] - Migrate com.facebook.react.modules.network.OkHttpClientProvider to Kotlin Pull Request resolved: #49108 Test Plan: ```bash yarn test-android yarn android ``` Reviewed By: cortinico Differential Revision: D69050956 Pulled By: javache fbshipit-source-id: 62dcf8e8f999f3b687c57ed02e9ac1f2db8183ea
Summary:
Currently, the class OkHttpClientProvider is still in Java. Adding some tests before migrating it to Kotlin
Changelog:
[INTERNAL] - Add OkHttpClientProvider tests
Test Plan: