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

fix: default OkHttpClient enhancements and testing #125

Merged
merged 8 commits into from
Dec 11, 2024

Conversation

Mohammad-Dwairi
Copy link
Contributor

Situation

With the introduction of the new SDK core, we need to raise the tests coverage before attempting to migrate the core module to the original repository. We will be testing the new core with this LC SDK during this temporary phase.

We pushed the new core initially as an PoC. However, we are revising each module to cleanup, improve, and test.

Task

  1. Cover all classes related to the default OkHttpClient in the new core with unit tests.
  2. Improve and cleanup related classes where needed.

Action

Code Enhancements

  1. Removed unneeded synchronization logic in BaseOkHttpClient that was used in the singleton pattern and relied on eager initialization of the default instance.
  2. Moved all mapping logic between OkHttp and SDK Transport to extension functions.
  3. Cleaned up OkHttpTransport implementation

Code Tests

  1. Covered all classes in com.expediagroup.sdk.core.okhttp package with Kotlin tests and Java tests if needed.

Testing

All tests are passing

Results

Screenshot 2024-12-09 at 16 41 07

@Mohammad-Dwairi Mohammad-Dwairi requested a review from a team as a code owner December 9, 2024 13:41
anssari1
anssari1 previously approved these changes Dec 10, 2024
@Mohammad-Dwairi Mohammad-Dwairi changed the title chore: default OkHttpClient enhancements and testing fix: default OkHttpClient enhancements and testing Dec 11, 2024
@Mohammad-Dwairi Mohammad-Dwairi merged commit 2fa398b into main Dec 11, 2024
1 check passed
@Mohammad-Dwairi Mohammad-Dwairi deleted the mdwairi/okhttp-unit-tests branch December 11, 2024 13:30
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.

3 participants