Skip to content
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
8 changes: 8 additions & 0 deletions .github/files/e2e-tests/e2e-matrix.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ const projects = [
suite: '',
buildGroup: 'jetpack-boost',
},
{
project: 'Jetpack Boost - Modules',
path: 'projects/plugins/boost/tests/e2e',
testArgs: [ 'specs/modules' ],
targets: [ 'plugins/boost' ],
suite: '',
buildGroup: 'jetpack-boost',
},
{
project: 'Jetpack Boost - Critical CSS',
path: 'projects/plugins/boost/tests/e2e',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,26 @@ const SpeedScore = () => {
}
}, [ loadScore, site.online ] );

const isCriticalCssEnabled = () => {
if ( ! data?.cloud_css?.available ) {
return data?.cloud_css?.active ?? false;
}

return data?.critical_css?.active ?? false;
};

// Refresh the score when something that can affect the score changes.
useDebouncedRefreshScore(
{
moduleStates,
pendingStates: {
criticalCss: {
isPending: cssState.status === 'pending' || criticalCssIsGenerating,
isPending:
isCriticalCssEnabled() && ( cssState.status === 'pending' || criticalCssIsGenerating ),
timestamp: cssState.updated || 0,
},
lcp: {
isPending: lcpState?.status === 'pending',
isPending: data?.lcp?.active === true && lcpState?.status === 'pending',
timestamp: lcpState?.updated || 0,
},
},
Expand Down
2 changes: 1 addition & 1 deletion projects/plugins/boost/app/lib/class-cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class CLI {
*/
private $jetpack_boost;

const MAKE_E2E_TESTS_WORK_MODULES = array( 'critical_css', 'render_blocking_js', 'page_cache', 'minify_js', 'minify_css', 'image_cdn', 'image_guide' );
const MAKE_E2E_TESTS_WORK_MODULES = array( 'critical_css', 'render_blocking_js', 'page_cache', 'lcp', 'minify_js', 'minify_css', 'image_cdn', 'image_guide' );

/**
* CLI constructor.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Add back previously flaky e2e tests


Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import WpPage from '_jetpack-e2e-commons/pages/wp-page.js';

const apiEndpointsRegex = {
'modules-state': /jetpack-boost-ds\/modules-state\/set/,
connection: /jetpack-boost\/v1\/connection/,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was never used.

};

export default class JetpackBoostPage extends WpPage {
Expand All @@ -18,8 +17,9 @@ export default class JetpackBoostPage extends WpPage {
async chooseFreePlan() {
const button = this.page.locator( 'text=Start for free' );
await button.click();

// We should wait a longer time to ensure the connection/plan is complete/established.
await this.waitForElementToBeVisible( '[data-testid="speed-scores"]', 30 * 1000 );
await this.isOverallScoreHeaderShown( 30 * 1000 );
}

/**
Expand Down Expand Up @@ -47,8 +47,8 @@ export default class JetpackBoostPage extends WpPage {
return showingScoreArea && ! isOffline;
}

async isOverallScoreHeaderShown() {
return await this.isElementVisible( '[data-testid="speed-scores"]' );
async isOverallScoreHeaderShown( timeout ) {
return await this.isElementVisible( '[data-testid="speed-scores"]', timeout );
}

async isSiteScoreLoading() {
Expand All @@ -57,27 +57,78 @@ export default class JetpackBoostPage extends WpPage {
return classNames.includes( 'loading' );
}

async waitForApiResponse( apiEndpointId ) {
async waitForApiResponse( apiEndpointId, moduleName, expectedState ) {
await this.page.waitForResponse(
response =>
response.url().match( apiEndpointsRegex[ apiEndpointId ] ) && response.status() === 200,
async response => {
const isSuccess = response.status() === 200;
if ( ! isSuccess ) {
return false;
}

const isMatch = response.url().match( apiEndpointsRegex[ apiEndpointId ] );
if ( ! isMatch ) {
return false;
}

const body = ( await response.json() )?.JSON;
console.log( `body[ ${ moduleName } ]?.active >`, body[ moduleName ]?.active );
console.log( 'expectedState >', expectedState );
return body[ moduleName ]?.active === expectedState;
},
{ timeout: 2 * 60 * 1000 }
);
}

async toggleModule( moduleName ) {
this.page.click( `.jb-feature-toggle-${ moduleName }` );
await this.waitForApiResponse( 'modules-state' );
/**
* Toggle a module and wait for the success notice to appear.
*
* @param {string} moduleName - The name of the module to toggle.
* @param {boolean} expectedState - The expected state of the module.
*/
async toggleModule( moduleName, expectedState ) {
console.log( `toggleModule > ${ moduleName } > ${ expectedState }` );

const stateSelector = expectedState ? ':not(.is-checked)' : '.is-checked';
const locator = `[data-testid="module-${ moduleName }"] .components-form-toggle${ stateSelector } input`;

const toggle = this.page.locator( locator );

toggle.click();

// Wait for the success notice to appear
const expectedMessage = expectedState ? 'Module activated' : 'Module deactivated';
const notice = this.page.locator( `.components-snackbar:has-text("${ expectedMessage }")` );
await notice.waitFor( {
timeout: 10000,
} );

// Wait for the notice to disappear
await notice.waitFor( {
timeout: 10000,
state: 'hidden',
} );
}

async isModuleEnabled( moduleName ) {
async waitForModuleState( moduleName, expectedState = true ) {
console.log( 'before >', expectedState );
const toggleSwitch = this.page.locator(
`.jb-feature-toggle-${ moduleName } .components-form-toggle`
);

// Wait for the toggle to reach the expected state
await toggleSwitch.waitFor();
const classNames = await toggleSwitch.getAttribute( 'class' );

return classNames.includes( 'is-checked' );
// Wait for the element to have the expected class state
await toggleSwitch.waitFor( async () => {
const classNames = await toggleSwitch.getAttribute( 'class' );
const isChecked = classNames.includes( 'is-checked' );
return expectedState ? isChecked : ! isChecked;
} );

// Return whether the expected state was achieved
const classNames = await toggleSwitch.getAttribute( 'class' );
const actualState = classNames.includes( 'is-checked' );
return actualState === expectedState;
}

async getSpeedScore( platform ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ test.describe( 'Critical CSS module', () => {
if ( previousTheme !== null ) {
await execWpCommand( `theme activate ${ previousTheme }` );
}
await page.close();
} );

// NOTE: The order of the following tests is important as we are making reuse of the generated Critical CSS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { JetpackBoostPage } from '../../lib/pages/index.js';

const modules = [
// ['MODULE_NAME', 'DEFAULT STATE'],
[ 'critical_css', 'disabled' ],
[ 'critical_css', 'enabled' ],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be enabled. We now enable local Critical CSS by default when the user selects a free plan.

[ 'render_blocking_js', 'disabled' ],
];

Expand All @@ -17,34 +17,40 @@ test.describe.serial( 'Modules', () => {
page = await browser.newPage( playwrightConfig.use );

await boostPrerequisitesBuilder( page )
.withCleanEnv()
.withConnection( true )
.withInactiveModules( [ 'critical_css', 'render_blocking_js' ] )
.withSpeedScoreMocked( true )
.build();
jetpackBoostPage = await JetpackBoostPage.visit( page );
} );

test.afterAll( async () => {
await page.close();
} );

modules.forEach( ( [ moduleSlug, moduleState ] = module ) => {
test( `The ${ moduleSlug } module should be ${ moduleState } by default`, async () => {
expect(
await jetpackBoostPage.isModuleEnabled( moduleSlug ),
await jetpackBoostPage.waitForModuleState( moduleSlug, moduleState === 'enabled' ),
`${ moduleSlug } should be enabled`
).toEqual( moduleState === 'enabled' );
).toBeTruthy();
} );

test( `The ${ moduleSlug } module state should toggle to an inverse state`, async () => {
await jetpackBoostPage.toggleModule( moduleSlug );
await jetpackBoostPage.toggleModule( moduleSlug, moduleState !== 'enabled' );
expect(
await jetpackBoostPage.isModuleEnabled( moduleSlug ),
`${ moduleSlug } should not be enabled`
).toEqual( moduleState !== 'enabled' );
await jetpackBoostPage.waitForModuleState( moduleSlug, moduleState !== 'enabled' ),
`${ moduleSlug } should be enabled`
).toBeTruthy();
} );

test( `The ${ moduleSlug } module state should revert back to original state`, async () => {
await jetpackBoostPage.toggleModule( moduleSlug );
await jetpackBoostPage.toggleModule( moduleSlug, moduleState === 'enabled' );

expect(
await jetpackBoostPage.isModuleEnabled( moduleSlug ),
await jetpackBoostPage.waitForModuleState( moduleSlug, moduleState === 'enabled' ),
`${ moduleSlug } should be enabled`
).toEqual( moduleState === 'enabled' );
).toBeTruthy();
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,16 @@ test.describe( 'Render Blocking JS module', () => {

test.beforeAll( async ( { browser } ) => {
page = await browser.newPage( playwrightConfig.use );
await boostPrerequisitesBuilder( page ).withTestContent( [ testPostTitle ] ).build();
await boostPrerequisitesBuilder( page )
.withCleanEnv()
.withMockConnection( true )
.withSpeedScoreMocked( true )
.withTestContent( [ testPostTitle ] )
.build();
} );

test.afterAll( async () => {
await page.close();
} );

test( 'JavaScript on a post should be at its original position in the document when the module is inactive', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,28 @@ test.describe( 'Auto refresh of speed scores', () => {
page = await browser.newPage( playwrightConfig.use );

await boostPrerequisitesBuilder( page )
.withCleanEnv()
.withConnection( true )
.withSpeedScoreMocked( false )
.withInactiveModules( [ 'critical_css', 'render_blocking_js' ] )
.build();
jetpackBoostPage = await JetpackBoostPage.visit( page );
} );

test.afterAll( async () => {
await page.close();
} );

[ 'render_blocking_js' ].forEach( moduleSlug => {
test( `Enabling ${ moduleSlug } should refresh scores`, async () => {
await jetpackBoostPage.waitForScoreLoadingToFinish();

await jetpackBoostPage.toggleModule( moduleSlug );
expect( await jetpackBoostPage.isScoreVisible(), 'Score should be visible' ).toBeTruthy();

await jetpackBoostPage.toggleModule( moduleSlug, true );

await new Promise( resolve => setTimeout( resolve, 2100 ) ); // Score refresh starts after 2 seconds delay

expect( await jetpackBoostPage.isScoreLoading(), 'Score should be loading' ).toBeTruthy();
await jetpackBoostPage.waitForScoreLoadingToFinish();
expect( await jetpackBoostPage.isScoreVisible(), 'Score should be visible' ).toBeTruthy();
} );
Expand All @@ -34,11 +41,13 @@ test.describe( 'Auto refresh of speed scores', () => {
test( 'Score refresh should debounce between multiple module toggle', async () => {
await jetpackBoostPage.waitForScoreLoadingToFinish();

expect( await jetpackBoostPage.isScoreVisible(), 'Score should be visible' ).toBeTruthy();

// Wait a second before toggling another.
await new Promise( resolve => setTimeout( resolve, 1000 ) );

// Toggle another module before the automatic score refresh started
const renderBlockingPromise = jetpackBoostPage.toggleModule( 'render_blocking_js' );
const renderBlockingPromise = jetpackBoostPage.toggleModule( 'minify_js', true );

// Wait slightly more than a second after second module is toggled
await new Promise( resolve => setTimeout( resolve, 1100 ) );
Expand Down
23 changes: 14 additions & 9 deletions projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,34 @@
import { boostPrerequisitesBuilder } from '../../lib/env/prerequisites.js';
import { JetpackBoostPage } from '../../lib/pages/index.js';

let jetpackBoostPage;

test.describe( 'Speed Score feature', () => {
let page;
let jetpackBoostPage;

test.beforeAll( async ( { browser } ) => {
const page = await browser.newPage();
await boostPrerequisitesBuilder( page ).withSpeedScoreMocked( false ).build();
page = await browser.newPage();
await boostPrerequisitesBuilder( page )
.withCleanEnv()
.withConnection( true )
.withSpeedScoreMocked( false )
.build();
} );

test.afterAll( async ( { browser } ) => {
const page = await browser.newPage();
await boostPrerequisitesBuilder( page ).withSpeedScoreMocked( true ).build();
test.afterAll( async () => {
await page.close();
} );

test.beforeEach( async function ( { page } ) {
test.beforeEach( async () => {

Check failure on line 22 in projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js

View workflow job for this annotation

GitHub Actions / Jetpack Boost - Modules e2e tests

[chromium] › specs/modules/speed-score.test.js:26:2 › Speed Score feature › The Speed Score section should display a mobile and desktop speed score greater than zero

1) [chromium] › specs/modules/speed-score.test.js:26:2 › Speed Score feature › The Speed Score section should display a mobile and desktop speed score greater than zero Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Test timeout of 300000ms exceeded while running "beforeEach" hook. 20 | } ); 21 | > 22 | test.beforeEach( async () => { | ^ 23 | jetpackBoostPage = await JetpackBoostPage.visit( page ); 24 | } ); 25 | at /home/runner/work/jetpack/jetpack/projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js:22:7
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This beforeEach simply just visits the Boost page, the fact that this times out tells me there is a timeout issue with localtunnel, a retry resolved this issue.
specs/modules/speed-score.test.js:26:2 › Speed Score feature › The Speed Score section should display a mobile and desktop speed score greater than zero Test timeout of 300000ms exceeded while running "beforeEach" hook. 20 | } ); 21 | > 22 | test.beforeEach( async () => { | ^ 23 | jetpackBoostPage = await JetpackBoostPage.visit( page ); 24 | } ); 25 | at /home/runner/work/jetpack/jetpack/projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js:22:7

jetpackBoostPage = await JetpackBoostPage.visit( page );
} );

test( 'The Speed Score section should display a mobile and desktop speed score greater than zero', async () => {
await jetpackBoostPage.waitForScoreLoadingToFinish();

expect(
await jetpackBoostPage.getSpeedScore( 'mobile' ),
'Mobile speed score should be greater than 0'
).toBeGreaterThan( 0 );

Check failure on line 32 in projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js

View workflow job for this annotation

GitHub Actions / Jetpack Boost - Modules e2e tests

[chromium] › specs/modules/speed-score.test.js:26:2 › Speed Score feature › The Speed Score section should display a mobile and desktop speed score greater than zero

1) [chromium] › specs/modules/speed-score.test.js:26:2 › Speed Score feature › The Speed Score section should display a mobile and desktop speed score greater than zero Error: Mobile speed score should be greater than 0 expect(received).toBeGreaterThan(expected) Expected: > 0 Received: 0 30 | await jetpackBoostPage.getSpeedScore( 'mobile' ), 31 | 'Mobile speed score should be greater than 0' > 32 | ).toBeGreaterThan( 0 ); | ^ 33 | expect( 34 | await jetpackBoostPage.getSpeedScore( 'desktop' ), 35 | 'Desktop speed score should be greater than 0' at /home/runner/work/jetpack/jetpack/projects/plugins/boost/tests/e2e/specs/modules/speed-score.test.js:32:5
expect(
await jetpackBoostPage.getSpeedScore( 'desktop' ),
'Desktop speed score should be greater than 0'
Expand All @@ -34,7 +40,6 @@
await jetpackBoostPage.waitForScoreLoadingToFinish();
await jetpackBoostPage.clickRefreshSpeedScore();

expect( await jetpackBoostPage.isScoreLoading(), 'Score should be loading' ).toBeTruthy();
await jetpackBoostPage.waitForScoreLoadingToFinish();
expect( await jetpackBoostPage.isScoreVisible(), 'Score should be displayed' ).toBeTruthy();
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ test.describe( 'Cache module', () => {
await permalinksPage.usePlainStructure();

const jetpackBoostPage = await JetpackBoostPage.visit( page );
await jetpackBoostPage.toggleModule( 'page_cache' );
await jetpackBoostPage.toggleModule( 'page_cache', true );
expect(
await jetpackBoostPage.waitForPageCachePermalinksErrorVisibility(),
'Page Cache should show permalink error message when using plain permalink structure'
Expand All @@ -89,7 +89,7 @@ test.describe( 'Cache module', () => {

// Activate the module.
const jetpackBoostPage = await JetpackBoostPage.visit( page );
await jetpackBoostPage.toggleModule( 'page_cache' );
await jetpackBoostPage.toggleModule( 'page_cache', true );

expect(
await jetpackBoostPage.waitForPageCacheMetaInfoVisibility(),
Expand Down
Loading