Embed block integration tests part 2 #35533
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
jd-alexander
added
the
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.08 MB ℹ️ View Unchanged
|
jd-alexander
commented
Oct 13, 2021
fluiddot
reviewed
Oct 13, 2021
…tion-tests' into rnmobile/add/embed-block-integration-tests-2
jd-alexander
commented
Oct 14, 2021
…tion-tests' into rnmobile/add/embed-block-integration-tests-2
jd-alexander
commented
Oct 15, 2021
…le/add/embed-block-integration-tests-2 # Conflicts: # packages/block-library/src/embed/test/index.native.js # test/native/__mocks__/@wordpress/react-native-aztec/index.js
|
@jd-alexander heads up that I updated this branch with the base branch and solve the conflicts in this commit. |
Thanks! |
…le/add/embed-block-integration-tests-2
jd-alexander
commented
Oct 27, 2021
| }, | ||
| } ); | ||
|
|
||
| fireEvent.press( |
There was a problem hiding this comment.
The idea here is that the Slash inserter should be visible once the text that has the / followed by Embed is entered. Therefore, we should be able to press the Embed block button within the Autocomplete UI.
There was a problem hiding this comment.
That's a great idea, I'm wondering if we could query by the Embed text instead of the a11y label, as the items in the slash inserter display a text label, wdyt?
There was a problem hiding this comment.
I applied this suggestion in this commit.
jd-alexander
commented
Oct 27, 2021
| await waitFor( () => getByA11yLabel( 'Embed block' ) ) | ||
| ); | ||
|
|
||
| const block = await waitFor( () => |
There was a problem hiding this comment.
Currently, the test is failing at this point. It seems the AutocompleteUI for the slash inserter is not being rendered based on the events being set to the paragraph block. This will be investigated.
There was a problem hiding this comment.
From an investigation standpoint, I realized that the getAutoCompleterUI located within the native implementation is not being called when the the onChange event is sent via fireEvent.
I am wondering if a prop is missing from the mocked RCTAztecView that needs to be implemented, so that it can be rendered.
There was a problem hiding this comment.
Once the aforementioned issue is resolved, I am going to be replicating the test setup here https://github.com/WordPress/gutenberg/blob/rnmobile/add/embed-block-integration-tests-2/packages/block-library/src/embed/test/index.native.js#L193 so that all the providers that are shown within the AutocompleteUI can be tested and verified.
There was a problem hiding this comment.
I am wondering if a prop is missing from the mocked RCTAztecView that needs to be implemented, so that it can be rendered.
Interesting, I'll investigate this further in case we need to add extra functionality in the mock.
There was a problem hiding this comment.
After digging into how the autocomplete works, I found out that we had to trigger a different event in the RichText component as well as set the paragraph block as the default block, which is required by the autocomplete logic (I'll add more details about this topic in the file changes).
There was a problem hiding this comment.
In the following commits are the changes for addressing this issue:
fluiddot
reviewed
Nov 2, 2021
| @@ -153,6 +177,7 @@ beforeEach( () => { | |||
| // Mock embed responses | |||
| mockEmbedResponses( [ | |||
| RICH_TEXT_EMBED_SUCCESS_RESPONSE, | |||
| RICH_TEXT_EMBED_ERROR_RESPONSE, | |||
There was a problem hiding this comment.
mockEmbedResponses uses the url property of the response to match the request's URL, so in this case, we shouldn't add RICH_TEXT_EMBED_ERROR_RESPONSE as its value is null.
There was a problem hiding this comment.
Alternatively, you could mock the response to the oEmbed endpoint with a null response within the "Cannot embed should be shown on the placeholder if EmbedPreview data is null" test case:
// Return null response for requests to oembed endpoint.
fetchRequest.mockImplementation( ( { path } ) => {
const isEmbedRequest = path.startsWith( '/oembed/1.0/proxy' );
return Promise.resolve( isEmbedRequest ? EMBED_NULL_RESPONSE : {} );
} );
There was a problem hiding this comment.
I've addressed the issue in this commit.
fluiddot
reviewed
Nov 2, 2021
fluiddot
reviewed
Nov 2, 2021
…le/add/embed-block-integration-tests-2
fluiddot
reviewed
Nov 2, 2021
fluiddot
approved these changes
Nov 2, 2021
There was a problem hiding this comment.
@jd-alexander from my side this PR is ready to merge into the main branch, although since I've made some changes in the recent commits, it would be great if you could do a last review, thanks 🙇 .
Thanks @fluiddot I will review these changes. |
I did a final review and the tests here are looking good. Thanks for the help in resolving the |
fluiddot
added a commit
that referenced
this pull request
Nov 4, 2021
* Add testID to embed bottom sheet * Export registerBlock from block library * [WIP] Add embed block integration tests * Fix set valid URL embed integration test * Add waitForElement to initialize editor helper * Add embed preview mocked styles * Add most used embed providers insertion tests * Add test cases for setting URL by tapping on block * Add mock embed responses helper * Add edit URL test cases * Add invalid URL test case * Refactor embed block integration tests * Mock RN clipboard library * Add auto-paste URL from clipboard test cases * Use snapshots and simplify integration tests * Add update test snapshots command * Add change alignment test case * Add retry test case * Add preview coming soon test cases * Mocked RCTAztecView to utilize an underlying TextInput. * Add testID to Android version of picker * Omit style prop in Aztec mock * Add paste URL to create embed test cases * Update snapshots due to mocking Aztec * Revert "Update snapshots due to mocking Aztec" This reverts commit 2114925. * Unmock react-native-aztec for some block tests * Remove commented code * Embed block integration tests part 2 (#35533) * added test for Embed block caption. * WIP * fixed unneeded diff change. * WIP block settings * Mocked RCTAztecView to utilize an underlying TextInput. * Fixed Embed block caption test issues. * Created test - toggle resize for smaller devices media settings * Added cannot embed test. * Removed unneeded test id. * WIP insert embed from slash inserter. * Mock fetch request in cannot embed test case * Trigger onSelectionChange event instead of onChange * Query slash inserter item by text * Add expected HTML to slash inserter test case * Mock autocomplete component styles * Set paragraph as default block * Add empty paragraph HTML constant * Add test suite for insert via slash inserter case * Update toggle responsive test case * Fix request mock for theme endpoint * Add slash inserter cases for most used providers * Expect for block settings button instead edit URL button * Use snapshot testing instead of checking HTML * Add block settings test suite * Add embed test snapshots * Use snapshot in insert generic embed block test Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Use promise in initializeEditor to prevent act warnings * Simplify tests using initializeWithEmbedBlock * Add test case to cover an already fixed bug * Add test case to cover an already fixed bug (#35013) Co-authored-by: Joel Dean <jdeanjj1000@gmail.com>
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
A continuation of #35476
How has this been tested?
Run npm run native test.
Observe that all tests pass. ( All tests don't currently pass due to an error.)
Screenshots
N/A
Types of changes
Tests
Checklist:
*.native.jsfiles for terms that need renaming or removal).