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

Add session replay assertions for internal testing #611

Merged

Conversation

louiszawadzki
Copy link
Contributor

What does this PR do?

Adds assertions for the Session Replay events.
For now we only assert on full snapshots, but that's enough to make sure the custom mappers we've added for RN session replay are working as intented.

For Android we had to add a bunch of improvements:

  • the detected padding was 1px off sometimes, so I added some flexibility into the assertions for padding
  • the default color was #0 instead of #000000FF, so I fixed it
  • the color was in lowercase vs uppercase for iOS, I formatted all colors to be uppercase

Motivation

Being able to run UI-level tests to better cover the Session Replay feature.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

@louiszawadzki louiszawadzki requested a review from a team as a code owner February 13, 2024 14:53
0xnm
0xnm previously approved these changes Feb 13, 2024
Copy link
Member

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

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

I didn't dig too much into TS/Swift code, but Kotlin part lgtm.

@@ -144,6 +144,12 @@ internal class ReactTextPropertiesResolver(
}

private fun getTextColor(shadowNodeWrapper: ShadowNodeWrapper): String? {
val isColorSet = shadowNodeWrapper
.getDeclaredShadowNodeField(IS_COLOR_SET_FIELD_NAME) as Boolean?
if (isColorSet != true) {
Copy link
Member

Choose a reason for hiding this comment

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

if it is not set, won't resolvedColor below be null? if it is the case, we can probably simply return #000000FF there directly, without querying another property here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is not set resolvedColor will actually return 0. It will return null if for some reason it cannot get access to the property through reflection

@louiszawadzki louiszawadzki force-pushed the louiszawadzki/rum-2348/add-session-replay-assertions branch from bd349cb to caf333f Compare February 13, 2024 16:45
@louiszawadzki louiszawadzki requested a review from 0xnm February 13, 2024 16:58
0xnm
0xnm previously approved these changes Feb 13, 2024
@louiszawadzki louiszawadzki force-pushed the louiszawadzki/rum-2348/add-session-replay-assertions branch from dc84f60 to 87bb011 Compare February 14, 2024 09:45
@louiszawadzki louiszawadzki requested a review from 0xnm February 14, 2024 13:03
@louiszawadzki louiszawadzki merged commit 6b99f64 into develop Feb 14, 2024
4 checks passed
@louiszawadzki louiszawadzki deleted the louiszawadzki/rum-2348/add-session-replay-assertions branch February 14, 2024 15:49
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