Skip to content
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

Add end 2 end tests InnerBlocks renderAppender #14996

Merged

Conversation

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Apr 16, 2019

Description

This PR adds end 2 end tests to renderAppender property of InnerBlocks added in #14241.

How has this been tested?

I checked end 2 end tests execute with success.

Copy link
Member

aduth left a comment

There's a failed test in the build in this newly added suite. It should be made to pass consistently.

@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch 2 times, most recently from f9c411d to da0c6a7 Apr 29, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch 2 times, most recently from e4967b5 to 9ce9b74 Oct 4, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch 2 times, most recently from a6225e9 to 271817e Oct 14, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Oct 14, 2019

The PR is rebased and is updated the tests are passing consistently. I think it is ready for a review.
@gziolo would you be able to have a look?

@jorgefilipecosta jorgefilipecosta dismissed aduth’s stale review Oct 14, 2019

Review was addressed.

@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch from 271817e to 6fe1c75 Nov 27, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Nov 27, 2019

Hi @aduth, this PR was rebased could you a new look?

/**
* Registers a custom script for the plugin.
*/
function enqueue_inner_blocks_renderappender_script() {

This comment has been minimized.

Copy link
@aduth

aduth Dec 4, 2019

Member

By its camelCase renderAppender, we should treat "render appender" as two words, reflecting this as well in its kebab-case and snake_case forms as render-appender.php and render_appender respectively.

var InnerBlocks = wp.blockEditor.InnerBlocks;
var withSelect = wp.data.withSelect;

var dataSelector = withSelect( function( select, ownProps ) {

This comment has been minimized.

Copy link
@aduth

aduth Dec 4, 2019

Member

Optional: Could use hooks form now.

openAllBlockInserterCategories,
} from '@wordpress/e2e-test-utils';

const QUOTE_INSERT_BUTTON_SELECTOR = '//button[contains(concat(" ", @class, " "), " block-editor-block-types-list__item ")][./span[contains(text(),"Quote")]]';

This comment has been minimized.

Copy link
@aduth

aduth Dec 4, 2019

Member

If it was really the case that the inner span selection ended up being an issue resolved in #18771, then I think we should be a bit more wary about it. I think there might be some options with XPath to traverse back up to the parent of a matched span, if we need to match it? https://devhints.io/xpath#more-examples

Generally, I kinda wish these elements were just easier to target. I'd almost be okay with adapting the output in the source code for the sake of the tests, if it makes them more reliable.

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Dec 6, 2019

Author Member

I updated the selection mechanism to use logic similar to what we used on #18771.

'Quote',
'Video',
] );
const quoteButton = ( await page.$x( QUOTE_INSERT_BUTTON_SELECTOR ) )[ 0 ];

This comment has been minimized.

Copy link
@aduth

aduth Dec 4, 2019

Member

Can we add some inline comments in this test case to explain the steps? As it stands, it's pretty difficult to follow.

@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch from 6fe1c75 to d6d57d3 Dec 6, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the add/end-2-end-test-innerBlocks-renderAppender branch from d6d57d3 to ddd783c Dec 6, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Dec 6, 2019

Hi @aduth, thank you for the review I applied changes that I hope to address all the points raised 👍

@aduth
aduth approved these changes Dec 16, 2019
@jorgefilipecosta jorgefilipecosta merged commit d0ae224 into master Dec 16, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@jorgefilipecosta jorgefilipecosta deleted the add/end-2-end-test-innerBlocks-renderAppender branch Dec 16, 2019
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.