-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[RNMobile] Audio block UI test #28674
Conversation
Size Change: 0 B Total Size: 1.36 MB ℹ️ View Unchanged
|
if ( isAndroid ) { | ||
await editorPage.driver.sleep( 5000 ); | ||
} else { | ||
await editorPage.driver.sleep( 1000 ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because there was some flakiness in the File Block test when the chooseMediaLibrary
option was used below. So I just adapted this here as a precaution. Let me know if that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a typo here and it's always truthy
in this case. isAndroid
is a function and should be called with isAndroid()
instead. Same for the File block, I hope tests won't fail on iOS after fixing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I resolved this for the File and Audio Block a1ed719. I will observe the iOS tests to see if they fail or pass and make the necessary modifications to get the code working as expected.
await editorPage.chooseMediaLibrary(); | ||
|
||
const html = await editorPage.getHtmlContent(); | ||
expect( testData.audioBlockPlaceholder.toLowerCase() ).toBe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I made a mistake converting these expectations lately. It should've been expect(received).toBe(expected)
for the diff in case of an error to display correctly.
expect( html... ).toBe(testData...)
in this case. Do you think it makes sense to fix other places in this PR as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! I resolved this for the File and Audio block since the changes being done in this PR were related to both blocks. I will create another PR for the block tests that need these assertions inverted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a PR for the rest of the expect
matcher fixes here #28770
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
Description
These changes involve adding UI tests for the Audio Block component on the mobile version of the editor.
How has this been tested?
CI should be ✅ on wordpress-mobile/gutenberg-mobile#3091
Types of changes
Checklist: