Add Cover block edit media integration tests #30270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcalhoun
added
Mobile App - i.e. Android or iOS
dcalhoun
force-pushed
the
add/cover-block-integration-tests
branch
2 times, most recently
from
12b0aaa to
41ff8a1
Compare
|
Size Change: 0 B Total Size: 1.46 MB ℹ️ View Unchanged
|
dcalhoun
commented
Mar 26, 2021
7 tasks
dcalhoun
force-pushed
the
add/cover-block-integration-tests
branch
from
aed977d to
068baa1
Compare
dcalhoun
changed the title
Previously, running Jest unit tests outputted a React warning because the mock was overly simplified and could not accept the refs passed to it as it is a function component.
Add integration test for Cover block controls.
Simple render test of Cover edit.
Superseded by Cover edit tests.
Update component to only set state if it is still "mounted."
These tests pass, but output errors in the test log due to an open issue in RNTL. https://git.io/JYYGE
Avoid erroneously removing an a11y label.
dcalhoun
force-pushed
the
add/cover-block-integration-tests
branch
from
8c9e976 to
f69c374
Compare
|
Awesome to see this evaluated! ❤️ And looking at the "Unit Tests / Mobile (pull request)" CI job, the execution of the test is quite speedy, most of the tests taking less than a second! 🎉 Any idea perhaps why the |
@hypest from my initial testing, it would appear to be related to the cache generated for the test suite and not the specific test itself. So, it is merely a penalty the first test in a given suite pays. I'm unsure if there is a way to improve it, but I can spend a little time investigating. No Media First
No Media Skipped
No Media Last
|
|
I explored the longer test run a little more. There doesn't appear to be one aspect of the rendered tree that causes the test to take longer. The biggest offender appears to the the |
|
@geriux would you be willing to review the approach for these Cover block edit tests? There are few changes to the components themselves. The ones of note might be the string change in |
Of course! will do 👍 |
geriux
reviewed
Apr 8, 2021
…-block-integration-tests
geriux
reviewed
Apr 14, 2021
There was a problem hiding this comment.
Thanks for adding this @dcalhoun!
I guess we'll have to keep an eye on the async issue for the tests that are being skipped now.
I don't know if you saw this but there's a warning for this Cover test:
console.warn
Non-serializable values were found in the navigation state. Check:
FocalPoint > params.onFocalPointChange (Function)
This can break usage such as persisting and restoring state. This might happen if you passed non-serializable values such as function, class instances etc.
@geriux yeah, unfortunately I believe that is the best path forward currently. The workarounds in the thread are fairly heavy handed (e.g. undoing a global
Yes, I was aware of this. It has actually been present and shows up at runtime since I merged #25810. The warning is also present for the Overlay Color nested bottom sheet for Cover blocks, which is why I chose to not address it at the time. Admittedly, it's a broken window of sorts. I am happy to repair it and have placed it on my todo list. Would you like me to repair the warning prior to merging this work or open up a follow up PR instead? |
I was about to reply after I tested the app and saw the warning was already there 😅 I think we can merge this and open a ticket to track that, what do you think? |
Yeah I saw those and I feel like we should just wait for an actual fix 👍 so having them as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add integration tests for the Cover block edit bottom sheet in the native editor. Specifically for the media settings. Sibling to #30306.
@testing-library/react-nativeto improve ability to test user-facing interface of components and align with web project guidelines of favoring RTL.TextInputinRangeTextInput.Future Test Coverage Opportunities
Svg.)BottomSheet.Cell.)PanRespondermay be complex.)How has this been tested?
npm run test-unit:nativeScreenshots
n/a
Types of changes
Automated tests
Checklist: