RUMM-2885 Add addFeatureFlagEvaluation function for RUM#1265
Conversation
`addFeatureFlagEvalutaion` adds the result of evaluating a feature flag to a view, which is key / value pair. These feature flags are sent with any subsequent View updates, as well as on Errors until the view is stopped.
Codecov Report
@@ Coverage Diff @@
## develop #1265 +/- ##
===========================================
- Coverage 82.01% 81.90% -0.11%
===========================================
Files 361 361
Lines 12832 12847 +15
Branches 2149 2151 +2
===========================================
- Hits 10524 10522 -2
- Misses 1655 1667 +12
- Partials 653 658 +5
|
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
lgtm! I've added some minor comments to the tests
| @StringForgery flagName: String, | ||
| @StringForgery flagValue: String | ||
| ) { | ||
| // GIVEN |
There was a problem hiding this comment.
Empty GIVEN. Should it be like that?
There was a problem hiding this comment.
All the givens here are set up by the fixture setup. I'm not sure if I should keep the comment and have a blank body or remove the comment?
There was a problem hiding this comment.
You can just remove it I think. They are all added manually, we don't have a template.
| argumentCaptor<ViewEvent> { | ||
| verify(mockWriter).write(eq(mockEventBatchWriter), capture()) | ||
| assertThat(lastValue) | ||
| .apply { |
There was a problem hiding this comment.
apply is not needed here if I'm not wrong, direct call is possible. Same for all tests below.
There was a problem hiding this comment.
👍 Yeah, I guess I only need apply if I were checking multiple things.
There was a problem hiding this comment.
it is not needed even for multiple assertions, because each assertion returns Assert instance, it can be called in the chain.
| } | ||
|
|
||
| // endregion | ||
|
|
There was a problem hiding this comment.
Perhaps missing a test on not sending pre-existing Feature Flag after a new view is started.
There was a problem hiding this comment.
Yeah, I'll probably need to see if I can add that to DatadogRumMonitorTest since that's what would handle changing the scope.
88eb385 to
25fa963
Compare
|
@0xnm Can I get a review of the new test before I merge in? |
5 participants
What does this PR do?
addFeatureFlagEvalutaionadds the result of evaluating a feature flag to a view, which is key / value pair. These feature flags are sent with any subsequent View updates, as well as on Errors until the view is stopped.Review checklist (to be filled by reviewers)