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

Extract logic for constructing ApiRequest from ApiService #61

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

fractalwrench
Copy link
Contributor

Goal

Extracts the logic for constructing ApiRequest objects into a separate file. This makes it easier to unit test the business logic for mapping events/sessions to ApiRequest.

This is necessary for future work that will give failed request types different priorities on the retry queue. This is mostly just a case of lifting functions into a separate source file, although I did remove/alter some redundant code in a couple of functions.

Testing

Added unit test coverage.

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #61 (a511645) into master (9034693) will increase coverage by 0.25%.
The diff coverage is 82.81%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   75.74%   75.99%   +0.25%     
==========================================
  Files         308      309       +1     
  Lines       10067    10057      -10     
  Branches     1452     1448       -4     
==========================================
+ Hits         7625     7643      +18     
+ Misses       1851     1826      -25     
+ Partials      591      588       -3     
Files Coverage Δ
...mbracesdk/comms/delivery/EmbraceDeliveryService.kt 60.83% <0.00%> (ø)
.../android/embracesdk/comms/api/EmbraceApiService.kt 82.05% <88.88%> (+5.35%) ⬆️
...e/android/embracesdk/comms/api/ApiRequestMapper.kt 82.22% <82.22%> (ø)

... and 2 files with indirect coverage changes

Copy link
Collaborator

@bidetofevil bidetofevil left a comment

Choose a reason for hiding this comment

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

Oh yeah this is nice. It'll work well with the changes I had and replace the clunky map I have!

@fractalwrench fractalwrench merged commit 5c13c0a into master Nov 14, 2023
4 checks passed
@fractalwrench fractalwrench deleted the refactor-api-service branch November 14, 2023 11:13
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.

2 participants