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

Comments Query Loop - Add e2e tests for the block #39502

Merged
merged 19 commits into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/e2e-test-utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### Enhancement

- Changed `setOption` to use `options.php`, to allow setting any option (and to be more consistent with `getOption`). [#39502](https://github.com/WordPress/gutenberg/pull/39502)
- Changed `setOption` to return the changed setting's previous value (to make restoring it easier). [#39502](https://github.com/WordPress/gutenberg/pull/39502)
- Added a new `trashAllComments` function.

## 7.0.0 (2022-03-11)

### Breaking Changes
Expand Down
12 changes: 12 additions & 0 deletions packages/e2e-test-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,10 @@ _Parameters_
- _setting_ `string`: The option, used to get the option by id.
- _value_ `string`: The value to set the option to.

_Returns_

- `string`: The previous value of the option.

### setPostContent

Sets code editor content
Expand Down Expand Up @@ -868,6 +872,14 @@ _Parameters_

- _name_ `string`: Block name.

### trashAllComments

Navigates to the comments listing screen and bulk-trashes any comments which exist.

_Returns_

- `Promise`: Promise resolving once comments have been trashed.

### trashAllPosts

Navigates to the post listing screen and bulk-trashes any posts which exist.
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export { toggleMoreMenu } from './toggle-more-menu';
export { toggleOfflineMode, isOfflineMode } from './offline-mode';
export { togglePreferencesOption } from './toggle-preferences-option';
export { transformBlockTo } from './transform-block-to';
export { trashAllComments } from './trash-all-comments';
export { uninstallPlugin } from './uninstall-plugin';
export { visitAdminPage } from './visit-admin-page';
export { waitForWindowDimensions } from './wait-for-window-dimensions';
Expand Down
14 changes: 11 additions & 3 deletions packages/e2e-test-utils/src/set-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,27 @@ import { pressKeyWithModifier } from './press-key-with-modifier';
*
* @param {string} setting The option, used to get the option by id.
* @param {string} value The value to set the option to.
*
* @return {string} The previous value of the option.
*
*/
export async function setOption( setting, value ) {
await switchUserToAdmin();
await visitAdminPage( 'options-general.php' );
await visitAdminPage( 'options.php' );

const previousValue = await page.$eval(
`#${ setting }`,
( element ) => element.value
);

await page.focus( `#${ setting }` );
await pressKeyWithModifier( 'primary', 'a' );
await page.type( `#${ setting }`, value );

await Promise.all( [
page.click( '#submit' ),
page.click( '#Update' ),
page.waitForNavigation( { waitUntil: 'networkidle0' } ),
] );

await switchUserToTest();
return previousValue;
}
35 changes: 35 additions & 0 deletions packages/e2e-test-utils/src/trash-all-comments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Internal dependencies
*/
import { switchUserToAdmin } from './switch-user-to-admin';
import { switchUserToTest } from './switch-user-to-test';
import { visitAdminPage } from './visit-admin-page';

/**
* Navigates to the comments listing screen and bulk-trashes any comments which exist.
*
* @return {Promise} Promise resolving once comments have been trashed.
*/
export async function trashAllComments() {
await switchUserToAdmin();
// Visit `/wp-admin/edit-comments.php` so we can see a list of comments and delete them.
await visitAdminPage( 'edit-comments.php' );

// If this selector doesn't exist there are no comments for us to delete.
const bulkSelector = await page.$( '#bulk-action-selector-top' );
if ( ! bulkSelector ) {
return;
}

// Select all comments.
await page.waitForSelector( '[id^=cb-select-all-]' );
await page.click( '[id^=cb-select-all-]' );
// Select the "bulk actions" > "trash" option.
await page.select( '#bulk-action-selector-top', 'trash' );
// Submit the form to send all mine/pendings/approved/spam comments to the trash.
await page.click( '#doaction' );
await page.waitForXPath(
'//*[contains(@class, "updated notice")]/p[contains(text(), "moved to the Trash.")]'
);
await switchUserToTest();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* WordPress dependencies
*/
import {
activateTheme,
createNewPost,
insertBlock,
pressKeyTimes,
publishPost,
setOption,
trashAllComments,
} from '@wordpress/e2e-test-utils';

describe( 'Comment Query Loop', () => {
let previousPageComments,
previousCommentsPerPage,
previousDefaultCommentsPage;
beforeAll( async () => {
await activateTheme( 'emptytheme' );
previousPageComments = await setOption( 'page_comments', '1' );
previousCommentsPerPage = await setOption( 'comments_per_page', '1' );
previousDefaultCommentsPage = await setOption(
'default_comments_page',
'newest'
);
} );
beforeEach( async () => {
await createNewPost();
} );
it( 'Pagination links are working as expected', async () => {
// Insert the Query Comment Loop block.
await insertBlock( 'Comments Query Loop' );
// Insert the Comment Loop form.
await insertBlock( 'Post Comments Form' );
await publishPost();
// Visit the post that was just published.
await page.click(
'.post-publish-panel__postpublish-buttons .is-primary'
);

// TODO: We can extract this into a util once we find we need it elsewhere.
// Create three comments for that post.
for ( let i = 0; i < 3; i++ ) {
await page.waitForSelector( 'textarea#comment' );
await page.click( 'textarea#comment' );
await page.type(
`textarea#comment`,
`This is an automated comment - ${ i }`
);
await pressKeyTimes( 'Tab', 1 );
await page.keyboard.press( 'Enter' );
await page.waitForNavigation();
}
Comment on lines +42 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's keep this here for now. We can consider extracting it into a util once we find that we need it elsewhere 👍
(Maybe add a comment to that effect? Like a // TODO: ...?)


// We check that there is a previous comments page link.
await page.waitForSelector( '.wp-block-comments-pagination-previous' );
Copy link
Contributor

@ockham ockham Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one minor issue with this: If the selector is not there, this will time out and throw an error -- i.e. we won't even make it to the assertion on the next line. This means that the assertion is kind of redundant.

So should we remove the assertion? No, since it's better to explicitly state our intent rather than implicitly.

Let's keep this as-is for now and merge it; I might file a follow-up to experiment with replacing the waitForSelector with waitForNavigation 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentative diff for later:

diff --git a/packages/e2e-tests/specs/experiments/blocks/comments-query.test.js b/packages/e2e-tests/specs/experiments/blocks/comments-query.test.js
index 55c6b4b07a..d3b8c880a9 100644
--- a/packages/e2e-tests/specs/experiments/blocks/comments-query.test.js
+++ b/packages/e2e-tests/specs/experiments/blocks/comments-query.test.js
@@ -48,12 +48,13 @@ describe( 'Comment Query Loop', () => {
 				`This is an automated comment - ${ i }`
 			);
 			await pressKeyTimes( 'Tab', 1 );
-			await page.keyboard.press( 'Enter' );
-			await page.waitForNavigation();
+			await Promise.all( [
+				page.keyboard.press( 'Enter' ),
+				page.waitForNavigation( { waitUntil: 'networkidle0' } ),
+			] );
 		}
 
 		// We check that there is a previous comments page link.
-		await page.waitForSelector( '.wp-block-comments-pagination-previous' );
 		expect(
 			await page.$( '.wp-block-comments-pagination-previous' )
 		).not.toBeNull();
@@ -61,21 +62,25 @@ describe( 'Comment Query Loop', () => {
 			await page.$( '.wp-block-comments-pagination-next' )
 		).toBeNull();
 
-		await page.click( '.wp-block-comments-pagination-previous' );
+		await Promise.all( [
+			page.click( '.wp-block-comments-pagination-previous' ),
+			page.waitForNavigation( { waitUntil: 'networkidle0' } ),
+		] );
 
 		// We check that there are a previous and a next link.
-		await page.waitForSelector( '.wp-block-comments-pagination-previous' );
-		await page.waitForSelector( '.wp-block-comments-pagination-next' );
 		expect(
 			await page.$( '.wp-block-comments-pagination-previous' )
 		).not.toBeNull();
 		expect(
 			await page.$( '.wp-block-comments-pagination-next' )
 		).not.toBeNull();
-		await page.click( '.wp-block-comments-pagination-previous' );
+
+		await Promise.all( [
+			page.click( '.wp-block-comments-pagination-previous' ),
+			page.waitForNavigation( { waitUntil: 'networkidle0' } ),
+		] );
 
 		// We check that there is only have a next link
-		await page.waitForSelector( '.wp-block-comments-pagination-next' );
 		expect(
 			await page.$( '.wp-block-comments-pagination-previous' )
 		).toBeNull();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR: #39818

expect(
await page.$( '.wp-block-comments-pagination-previous' )
).not.toBeNull();
expect(
await page.$( '.wp-block-comments-pagination-next' )
).toBeNull();

await page.click( '.wp-block-comments-pagination-previous' );

// We check that there are a previous and a next link.
await page.waitForSelector( '.wp-block-comments-pagination-previous' );
await page.waitForSelector( '.wp-block-comments-pagination-next' );
expect(
await page.$( '.wp-block-comments-pagination-previous' )
).not.toBeNull();
expect(
await page.$( '.wp-block-comments-pagination-next' )
).not.toBeNull();
await page.click( '.wp-block-comments-pagination-previous' );

// We check that there is only have a next link
await page.waitForSelector( '.wp-block-comments-pagination-next' );
expect(
await page.$( '.wp-block-comments-pagination-previous' )
).toBeNull();
expect(
await page.$( '.wp-block-comments-pagination-next' )
).not.toBeNull();
} );
afterAll( async () => {
await trashAllComments();
await activateTheme( 'twentytwentyone' );
await setOption( 'page_comments', previousPageComments );
await setOption( 'comments_per_page', previousCommentsPerPage );
await setOption( 'default_comments_page', previousDefaultCommentsPage );
} );
} );
35 changes: 35 additions & 0 deletions phpunit/class-block-library-comment-template-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,39 @@ function test_rendering_comment_template_nested() {
'<ol ><li><div class="wp-block-comment-author-name">Test</div><div class="wp-block-comment-content">Hello world</div><ol><li><div class="wp-block-comment-author-name">Test</div><div class="wp-block-comment-content">Hello world</div><ol><li><div class="wp-block-comment-author-name">Test</div><div class="wp-block-comment-content">Hello world</div></li></ol></li></ol></li></ol>'
);
}
/**
* Test that both "Older Comments" and "Newer Comments" are displayed in the correct order
* inside the Comment Query Loop when we enable pagination on Discussion Settings.
* In order to do that, it should exist a query var 'cpage' set with the $comment_args['paged'] value.
*/
Comment on lines +166 to +170
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH without knowing much about build_comment_query_vars_from_block, pagination settings, and the cpage query arg and paged value, this is a lot to swallow 😅 It's not obvious to me how all these things are related to the correct order of "Older Comments" and "Newer Comments".

The test may have its value in covering the underlying implementation to ensure the desired behavior, but maybe it'd actually make sense to still add the e2e test to cover that behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function test_build_comment_query_vars_from_block_sets_cpage_var() {

// This could be any number, we set a fixed one instead of a random for better performance.
$comment_query_max_num_pages = 5;
// We substract 1 because we created 1 comment at the beggining.
$post_comments_numbers = ( self::$per_page * $comment_query_max_num_pages ) - 1;
self::factory()->comment->create_post_comments(
self::$custom_post->ID,
$post_comments_numbers,
array(
'comment_author' => 'Test',
'comment_author_email' => 'test@example.org',
'comment_content' => 'Hello world',
)
);
$parsed_blocks = parse_blocks(
'<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
);

$block = new WP_Block(
$parsed_blocks[0],
array(
'postId' => self::$custom_post->ID,
'comments/inherit' => true,
)
);
$actual = build_comment_query_vars_from_block( $block );
$this->assertEquals( $actual['paged'], $comment_query_max_num_pages );
$this->assertEquals( get_query_var( 'cpage' ), $comment_query_max_num_pages );
}
}