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 a plugin to disable the CSS animation for more stable e2e tests #13769

Merged
merged 1 commit into from
Feb 8, 2019
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
4 changes: 4 additions & 0 deletions packages/e2e-test-utils/src/activate-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import { visitAdminPage } from './visit-admin-page';
export async function activatePlugin( slug ) {
await switchUserToAdmin();
await visitAdminPage( 'plugins.php' );
const disableLink = await page.$( `tr[data-slug="${ slug }"] .deactivate a` );
if ( disableLink ) {
return;
}
await page.click( `tr[data-slug="${ slug }"] .activate a` );
await page.waitForSelector( `tr[data-slug="${ slug }"] .deactivate a` );
await switchUserToTest();
Expand Down
6 changes: 0 additions & 6 deletions packages/e2e-test-utils/src/click-on-more-menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
*/
import { first } from 'lodash';

/**
* Internal dependencies
*/
import { waitForAnimation } from './wait-for-animation';

/**
* Clicks on More Menu item, searches for the button with the text provided and clicks it.
*
Expand All @@ -17,7 +12,6 @@ export async function clickOnMoreMenuItem( buttonLabel ) {
await expect( page ).toClick(
'.edit-post-more-menu [aria-label="Show more tools & options"]'
);
await waitForAnimation();
const moreMenuContainerSelector =
'//*[contains(concat(" ", @class, " "), " edit-post-more-menu__content ")]';
let elementToClick = first( await page.$x(
Expand Down
1 change: 0 additions & 1 deletion packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export { toggleScreenOption } from './toggle-screen-option';
export { transformBlockTo } from './transform-block-to';
export { uninstallPlugin } from './uninstall-plugin';
export { visitAdminPage } from './visit-admin-page';
export { waitForAnimation } from './wait-for-animation';
export { waitForWindowDimensions } from './wait-for-window-dimensions';

export * from './mocks';
6 changes: 0 additions & 6 deletions packages/e2e-test-utils/src/search-for-block.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
/**
* Internal dependencies
*/
import { waitForAnimation } from './wait-for-animation';

/**
* Search for block in the global inserter
*
* @param {string} searchTerm The text to search the inserter for.
*/
export async function searchForBlock( searchTerm ) {
await page.click( '.edit-post-header [aria-label="Add block"]' );
await waitForAnimation();
// Waiting here is necessary because sometimes the inserter takes more time to
// render than Puppeteer takes to complete the 'click' action
await page.waitForSelector( '.editor-inserter__menu' );
Expand Down
6 changes: 0 additions & 6 deletions packages/e2e-test-utils/src/switch-editor-mode-to.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* Internal dependencies
*/
import { waitForAnimation } from './wait-for-animation';

/**
* Switches editor mode.
*
Expand All @@ -12,7 +7,6 @@ export async function switchEditorModeTo( mode ) {
await page.click(
'.edit-post-more-menu [aria-label="Show more tools & options"]'
);
await waitForAnimation();
const [ button ] = await page.$x(
`//button[contains(text(), '${ mode } Editor')]`
);
Expand Down
6 changes: 0 additions & 6 deletions packages/e2e-test-utils/src/transform-block-to.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* Internal dependencies
*/
import { waitForAnimation } from './wait-for-animation';

/**
* Converts editor's block type.
*
Expand All @@ -12,7 +7,6 @@ export async function transformBlockTo( name ) {
await page.mouse.move( 200, 300, { steps: 10 } );
await page.mouse.move( 250, 350, { steps: 10 } );
await page.click( '.editor-block-switcher__toggle' );
await waitForAnimation();
await page.waitForSelector( `.editor-block-types-list__item[aria-label="${ name }"]` );
await page.click( `.editor-block-types-list__item[aria-label="${ name }"]` );
}
10 changes: 0 additions & 10 deletions packages/e2e-test-utils/src/wait-for-animation.js

This file was deleted.

2 changes: 2 additions & 0 deletions packages/e2e-tests/config/setup-test-framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
enablePageDialogAccept,
setBrowserViewport,
visitAdminPage,
activatePlugin,
} from '@wordpress/e2e-test-utils';

/**
Expand Down Expand Up @@ -154,6 +155,7 @@ beforeAll( async () => {

await trashExistingPosts();
await setupBrowser();
await activatePlugin( 'gutenberg-test-plugin-disables-the-css-animations' );
Copy link
Member

Choose a reason for hiding this comment

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

Should it be placed inside mu-plugins instead? We have there one plugin already:

https://github.com/WordPress/gutenberg/blob/master/packages/e2e-tests/mu-plugins/disable-login-autofocus.php

Copy link
Member

Choose a reason for hiding this comment

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

Although, it might be a bad idea as it would change the behavior when using Docker outside of e2e context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was exactly my thinking :)

Copy link
Member

Choose a reason for hiding this comment

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

Unless we disable this line in docker config:
https://github.com/WordPress/gutenberg/blob/master/docker-compose.yml#L16

Sorry about all notifications :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the e2e tests use this docker config though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the e2e tests use a different container but when you run them locally, we often run them on the default container to test. (that's what I do), and they'll fail because animations are enabled there.

} );

afterEach( async () => {
Expand Down
18 changes: 18 additions & 0 deletions packages/e2e-tests/plugins/disable-animations.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
/**
* Plugin Name: Gutenberg Test Plugin, Disables the CSS animations
* Plugin URI: https://github.com/WordPress/gutenberg
* Author: Gutenberg Team
*
* @package gutenberg-test-disable-animations
*/

/**
* Enqueue CSS stylesheet disabling animations.
*/
function enqueue_disable_animations_stylesheet() {
$custom_css = '* { animation-duration: 1ms !important; }';
Copy link
Member

Choose a reason for hiding this comment

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

That's aggressive but I like it :)

Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why does this have to be 1ms and not 0? It seems we leave open the potential for race conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me if we do 0 it means we disable animations entirely and 1ms is saying the animation are there (to match prod) but are very fast. Not sure if it's a great argument though :P

Copy link
Member

Choose a reason for hiding this comment

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

That seems to me like the worst of both worlds though, we're not really testing the animation nor are we disabling them entirely.

Copy link
Member

Choose a reason for hiding this comment

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

See #13779. 0ms seems to be even better in my testing.

wp_add_inline_style( 'wp-components', $custom_css );
}

add_action( 'enqueue_block_editor_assets', 'enqueue_disable_animations_stylesheet' );
2 changes: 0 additions & 2 deletions packages/e2e-tests/specs/block-deletion.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
getEditedPostContent,
createNewPost,
pressKeyWithModifier,
waitForAnimation,
} from '@wordpress/e2e-test-utils';

const addThreeParagraphsToNewPost = async () => {
Expand All @@ -22,7 +21,6 @@ const addThreeParagraphsToNewPost = async () => {

const clickOnBlockSettingsMenuItem = async ( buttonLabel ) => {
await expect( page ).toClick( '.editor-block-settings-menu__toggle' );
await waitForAnimation();
const itemButton = ( await page.$x( `//*[contains(@class, "editor-block-settings-menu__popover")]//button[contains(text(), '${ buttonLabel }')]` ) )[ 0 ];
await itemButton.click();
};
Expand Down
3 changes: 0 additions & 3 deletions packages/e2e-tests/specs/block-hierarchy-navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ import {
getEditedPostContent,
pressKeyTimes,
pressKeyWithModifier,
waitForAnimation,
} from '@wordpress/e2e-test-utils';

async function openBlockNavigator() {
await pressKeyWithModifier( 'access', 'o' );
await waitForAnimation();
}

describe( 'Navigating the block hierarchy', () => {
Expand All @@ -28,7 +26,6 @@ describe( 'Navigating the block hierarchy', () => {

// Navigate to the columns blocks.
await page.click( '[aria-label="Block Navigation"]' );
await waitForAnimation();
const columnsBlockMenuItem = ( await page.$x( "//button[contains(@class,'editor-block-navigation__item') and contains(text(), 'Columns')]" ) )[ 0 ];
await columnsBlockMenuItem.click();

Expand Down
5 changes: 0 additions & 5 deletions packages/e2e-tests/specs/editor-modes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
clickBlockAppender,
createNewPost,
switchEditorModeTo,
waitForAnimation,
} from '@wordpress/e2e-test-utils';

describe( 'Editing modes (visual/HTML)', () => {
Expand All @@ -26,7 +25,6 @@ describe( 'Editing modes (visual/HTML)', () => {
// Change editing mode from "Visual" to "HTML".
await page.waitForSelector( 'button[aria-label="More options"]' );
await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();
let changeModeButton = await page.waitForXPath( '//button[text()="Edit as HTML"]' );
await changeModeButton.click();

Expand All @@ -40,7 +38,6 @@ describe( 'Editing modes (visual/HTML)', () => {
// Change editing mode from "HTML" back to "Visual".
await page.waitForSelector( 'button[aria-label="More options"]' );
await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();
changeModeButton = await page.waitForXPath( '//button[text()="Edit visually"]' );
await changeModeButton.click();

Expand All @@ -56,7 +53,6 @@ describe( 'Editing modes (visual/HTML)', () => {
// Change editing mode from "Visual" to "HTML".
await page.waitForSelector( 'button[aria-label="More options"]' );
await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();
const changeModeButton = await page.waitForXPath( '//button[text()="Edit as HTML"]' );
await changeModeButton.click();

Expand All @@ -73,7 +69,6 @@ describe( 'Editing modes (visual/HTML)', () => {
// Change editing mode from "Visual" to "HTML".
await page.waitForSelector( 'button[aria-label="More options"]' );
await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();
const changeModeButton = await page.waitForXPath( '//button[text()="Edit as HTML"]' );
await changeModeButton.click();

Expand Down
2 changes: 0 additions & 2 deletions packages/e2e-tests/specs/font-size-picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
getEditedPostContent,
createNewPost,
pressKeyTimes,
waitForAnimation,
} from '@wordpress/e2e-test-utils';

describe( 'Font Size Picker', () => {
Expand All @@ -19,7 +18,6 @@ describe( 'Font Size Picker', () => {
await clickBlockAppender();
await page.keyboard.type( 'Paragraph to be made "large"' );
await page.click( '.components-font-size-picker__selector' );
await waitForAnimation();
const changeSizeButton = await page.waitForSelector( '.components-button.is-font-large' );
await changeSizeButton.click();

Expand Down
2 changes: 0 additions & 2 deletions packages/e2e-tests/specs/invalid-block.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import {
createNewPost,
clickBlockAppender,
waitForAnimation,
} from '@wordpress/e2e-test-utils';

describe( 'invalid blocks', () => {
Expand All @@ -20,7 +19,6 @@ describe( 'invalid blocks', () => {
// Click the 'more options'
await page.mouse.move( 200, 300, { steps: 10 } );
await page.click( 'button[aria-label="More options"]' );
await waitForAnimation();

// Change to HTML mode and close the options
const changeModeButton = await page.waitForXPath( '//button[text()="Edit as HTML"]' );
Expand Down
Loading