[PM-10600] Unit Test coverage and refactor for Push Notifications #5011
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also added HttpClientCustomize fixture, that provides IHttpClientFactory, HttpClient and MockedHttpMessageHandler. The latter can be used to mock requests and provide expected responses. Expanded HttpRequestMatcher to be able to respond with content.
…ements. The GlobalSettingsFactory no longer provides the same instance of GlobalSettings. If modified, other tests would fail too. The factory now have a method to create new GlobalSettings objects, which can be modified as it's needed for the test. This is then used in GlobalSettingsBuilder, which can be used to customize GlobalSettings.
The GlobalSettings fixture customization had the same name as the GlobalSettings itself, causing a bunch of compilation issues.
…nce. The IHttpClientFactory.CreateClient(string name) needs to return different HttpClient instances in case there are multiple clients to be created. This have been issue for BaseIdentityClientService where the HttpClient is modified with BaseAddress, causing issues.
The SutProviderCustomization now includes create boolean flag, set to false, does not implicitly create the SutProvider instance. This gives flexibility to set dependencies and modify them, before the constructor of the Sut is called.
…dentityClientService SendAsync in BaseIdentityClientService never throws exception. Ideally this behaviour should be changed, and the method should throw error on any failure, but it's not in scope of change. Will be fixed by PM-14675
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## km/pm-10600 #5011 +/- ##
===============================================
+ Coverage 42.78% 43.03% +0.25%
===============================================
Files 1391 1389 -2
Lines 64902 64869 -33
Branches 5960 5960
===============================================
+ Hits 27769 27918 +149
+ Misses 35903 35714 -189
- Partials 1230 1237 +7 ☔ View full report in Codecov by Sentry. |
|
New Issues
Fixed Issues
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-10600
📔 Objective
Part 2 of Push Notification changes. This one focuses on refactoring for ease of testing.
More coverage and changes coming in Part 3.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes