Archive notifier tests #619
Conversation
…nality Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com>
Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com>
Codecov Report
@@ Coverage Diff @@
## master #619 +/- ##
==========================================
+ Coverage 92.88% 93.42% +0.54%
==========================================
Files 227 227
Lines 5902 5902
Branches 1487 1486 -1
==========================================
+ Hits 5482 5514 +32
+ Misses 379 347 -32
Partials 41 41
Continue to review full report at Codecov.
|
| @@ -73,7 +66,7 @@ function updateNotification(oldState: ENotifiedState | null, nextState: ENotifie | |||
| const { acknowledge, archivedState } = props; | |||
| if (nextState === ENotifiedState.Outcome) { | |||
| if (archivedState && archivedState.error) { | |||
| const error = typeof archivedState.error === 'string' ? archivedState.error : archivedState.error; | |||
| const { error } = archivedState; | |||
There was a problem hiding this comment.
what changed that makes it unnecessary to check for string type?
There was a problem hiding this comment.
No change, if you look at the true and false side of the ternary, the true and false condition are the same.
? archivedState.error : archivedState.error
archivedState.error could be anything or nothing and this if statement will always return archievedState.error.
If I had to guess, this code might predate the partial adoption of typescript, typescript's transpiler will break if you put an inappropriate value in the error property. There might have been a check here that got refactored, but at current, it's an unnecessary code branch.
There was a problem hiding this comment.
Just checked, this line was added in #194, it's always been this way. Probably just something that wasn't easy to spot without tests, hard to speculate.
* adding test coverage around current ArchiveNotifier component functionality Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> * clean up some outdated comments and remove some erroneous logic Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* adding test coverage around current ArchiveNotifier component functionality Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> * clean up some outdated comments and remove some erroneous logic Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* adding test coverage around current ArchiveNotifier component functionality Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> * clean up some outdated comments and remove some erroneous logic Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Which problem is this PR solving?
Short description of the changes