-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[NoQA] test: add support for transitionEnd event in reassure tests #33286
[NoQA] test: add support for transitionEnd event in reassure tests #33286
Conversation
Left one comment. Besides LGTM. |
@hurali97 It seems the |
@pecanoro Reassure tests are failing due to the output being different by render count. I also mentioned this in the PR description that we are now accounting for This doesn't happen on main because we were not accounting for ![]() |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb Safari |
@pecanoro looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
Not an emergency, the failure is expected since we are fixing perf tests. |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/pecanoro in version: 1.4.19-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.19-2 🚀
|
Details
This fixes the failing unit test on PR: #32336
The reasoning is that since in the attached PR Reassure test is failing due to the fact that we have now updated the condition to show
ReportActionsSkeletonView
andReportActionsView
based ondidScreenTransitionEnd
which is a state value and it's toggled in thetransitionEnd
event listener of navigation.Now, in Reassure we are just testing the mount of the
ReportScreen
so there's no actual navigation going on behind the scenes. Hence, thetransitionEnd
event doesn't fire and we always get thedidScreenTransitionEnd
as false. So reassure test fails since they can't findreport-actions-list
in the tree.So I have passed in the custom callback to trigger the
addListener
manually. Also, doing so requires us to use the navigation from prop instead ofuseNavigation
inScreenWrapper
.Previously, this wasn't causing any failures due to the fact that we were not leveraging
didScreenTransitionEnd
inReportScreen
. This is however used inSearchPage
,RoomInvitePage
and in other places, which might not be affected here as we don't have Reassure tests for them.Here, we are accounting for
ReportScreen
reassure test and it will produce Reassure output which is expected to be different from main.Fixed Issues
$ #32336
PROPOSAL: #32336
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
N/A
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A
MacOS: Desktop
N/A