AccessibilityInfo: added missing tests for Promise based methods, align null checks and error message code style #56150
AccessibilityInfo: added missing tests for Promise based methods, align null checks and error message code style #56150chicio wants to merge 15 commits into
Conversation
…vertColorsEnabled
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
|
Hi @vzaidman, maybe you can help me to import also this one? Thank you so much 🙏 |
…accessibility_missing_tests
|
Hi @cortinico, can you help me with this PR? Thanks 🙏 |
cortinico
left a comment
cortinico
left a comment
There was a problem hiding this comment.
Generally looks good to me, but left a couple of nits 👍
| const isReduceMotionEnabled = | ||
| await AccessibilityInfo.isReduceMotionEnabled(); | ||
|
|
||
| expect(isReduceMotionEnabled).toBe(true); |
There was a problem hiding this comment.
Can you also assert this?
expect(mockGetCurrentReduceMotionState).toHaveBeenCalled()
| const nativeAccessibilityInfoModule = jest.requireMock( | ||
| '../NativeAccessibilityInfo', | ||
| ); | ||
| nativeAccessibilityInfoModule.default = null; |
There was a problem hiding this comment.
This is different from the test title: 'should throw error if isReduceMotionEnabled is not available'
You should mockNativeAccessibilityInfoAndroid.isReduceMotionEnabled = null;
instead
There was a problem hiding this comment.
Actually the test is correct, but the title wrong.
I fixed it to match the naming used in similar tests that is should throw error if NativeAccessibilityInfoAndroid is not available.
Thanks 🙏 I addressed all the comment. Let me know if you spot anything else or it is ready to be merged. |
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this in D102190403. |
|
This pull request was successfully merged by @chicio in 8fc503d When will my fix make it into a release? | How to file a pick request? |
3 participants
Summary:
This PR continue the work I'm doing to improve the
AccessibilityInfomodule (#56066 and connected PRs).In this new PR I added the missing tests for all the Promise based methods in the
AccessibilityInfo, to prevent regressions similar to those fixed in the previous PRs.This PR also includes a couple of small code style consistency improvement:
getRecommendedTimeoutMillisto the other ones already present in the fileisGrayscaleEnabled,isInvertColorsEnabledandisReduceMotionEnabledto the other onesChangelog:
[GENERAL] [ADDED] - AccessibilityInfo: added missing tests for Promise based methods, align null checks and error message code style
Test Plan:
I run the tests that I added to verify their correctness. Some of them were failing because of the change of the error messages, and I made them green by changing the message in the production code. For this others, I verified their correctness by breaking the production code to see if they were failing.