Skip to content

Commit

Permalink
Merge pull request #10048 from google/e2e-debug
Browse files Browse the repository at this point in the history 
Fix flaky E2E tests
  • Loading branch information
@techanvil
techanvil authored Jan 18, 2025
2 parents 1e6163b + 6c4455f commit 944cad1
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 145 deletions.
36 changes: 36 additions & 0 deletions tests/e2e/mu-plugins/e2e-rest-time-endpoint.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/**
* Plugin Name: E2E Time Endpoint
* Description: REST Endpoint for supporting E2E tests.
*
* @package Google\Site_Kit
* @copyright 2025 Google LLC
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
* @link https://sitekit.withgoogle.com
*/

use Google\Site_Kit\Core\REST_API\REST_Routes;

add_action(
'rest_api_init',
function () {
if ( ! defined( 'GOOGLESITEKIT_PLUGIN_MAIN_FILE' ) ) {
return;
}

register_rest_route(
REST_Routes::REST_ROOT,
'e2e/util/data/time', // Required for use with SK API.
array(
'methods' => WP_REST_Server::READABLE,
'callback' => function () {
return array(
'time' => time(),
'microtime' => microtime(),
);
},
'permission_callback' => '__return_true',
)
);
}
);
18 changes: 0 additions & 18 deletions tests/e2e/plugins/module-setup-analytics-no-account.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,6 @@
add_action(
'rest_api_init',
function () {

register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics/data/accounts-properties-profiles',
array(
'methods' => 'GET',
'callback' => function () {
return array(
'accounts' => array(),
'properties' => array(),
'profiles' => array(),
);
},
'permission_callback' => '__return_true',
),
true
);

register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics-4/data/account-summaries',
Expand Down
98 changes: 29 additions & 69 deletions tests/e2e/plugins/module-setup-analytics.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
namespace Google\Site_Kit\Tests\E2E\Modules\Analytics;

use Google\Site_Kit\Core\REST_API\REST_Routes;
use Google\Site_Kit_Dependencies\Google\Service\TagManager\Destination;

const ACCOUNT_ID_A = '100';
const ACCOUNT_ID_B = '101';
Expand Down Expand Up @@ -85,25 +86,6 @@ function ( $item ) use ( $property_ids ) {
add_action(
'rest_api_init',
function () {
$accounts = array(
array(
'id' => ACCOUNT_ID_A,
'kind' => 'analytics#account',
'name' => 'Test Account A',
'permissions' => array(
'effective' => array( 'COLLABORATE', 'EDIT', 'MANAGE_USERS', 'READ_AND_ANALYZE' ),
),
),
array(
'id' => ACCOUNT_ID_B,
'kind' => 'analytics#account',
'name' => 'Test Account B',
'permissions' => array(
'effective' => array( 'COLLABORATE', 'EDIT', 'MANAGE_USERS', 'READ_AND_ANALYZE' ),
),
),
);

$account_summaries = array(
array(
'account' => 'accounts/' . ACCOUNT_ID_A,
Expand All @@ -113,6 +95,7 @@ function () {
'propertySummaries' => array(
array(
'displayName' => 'Example Property',
'parent' => 'account/' . ACCOUNT_ID_A,
'property' => 'properties/' . GA4_PROPERTY_ID_X,
'_id' => GA4_PROPERTY_ID_X,
),
Expand All @@ -126,6 +109,7 @@ function () {
'propertySummaries' => array(
array(
'displayName' => 'Example Property',
'parent' => 'account/' . ACCOUNT_ID_B,
'property' => 'properties/' . GA4_PROPERTY_ID_Y,
'_id' => GA4_PROPERTY_ID_Y,
),
Expand All @@ -139,6 +123,7 @@ function () {
'propertySummaries' => array(
array(
'displayName' => 'Example Property Z',
'parent' => 'account/' . ACCOUNT_ID_C,
'property' => 'properties/' . GA4_PROPERTY_ID_Z,
'_id' => GA4_PROPERTY_ID_Z,
),
Expand Down Expand Up @@ -202,25 +187,6 @@ function () {
),
);

register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics/data/accounts-properties-profiles',
array(
'methods' => 'GET',
'callback' => function () use ( $accounts ) {
$response = array(
'accounts' => $accounts,
'properties' => array(),
'profiles' => array(),
);

return $response;
},
'permission_callback' => '__return_true',
),
true
);

register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics-4/data/account-summaries',
Expand All @@ -237,37 +203,6 @@ function () {
true
);

// Called when switching accounts
register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics/data/properties-profiles',
array(
'methods' => 'GET',
'callback' => function () {
return array(
'properties' => array(),
'profiles' => array(),
);
},
'permission_callback' => '__return_true',
),
true
);

// Called when switching properties
register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics/data/profiles',
array(
'methods' => 'GET',
'callback' => function () {
return array();
},
'permission_callback' => '__return_true',
),
true
);

// Called when switching properties for Analytics 4
register_rest_route(
REST_Routes::REST_ROOT,
Expand Down Expand Up @@ -337,6 +272,31 @@ function () {
true
);

register_rest_route(
REST_Routes::REST_ROOT,
'modules/analytics-4/data/container-destinations',
array(
'methods' => 'GET',
'callback' => function ( \WP_REST_Request $request ) {
$account_id = $request->get_param( 'accountID' );
$container_id = $request->get_param( 'containerID' );
$destination = new Destination();
$destination->setPath( "accounts/$account_id/containers/$container_id/destinations/1" );
$destination->setAccountId( $account_id );
$destination->setContainerId( $container_id );
$destination->setDestinationLinkId( '1' );
$destination->setDestinationId( 'G-000' ); // Matches nothing here currently.
$destination->setName( 'Non-existent destination' );

return array(
$destination,
);
},
'permission_callback' => '__return_true',
),
true
);

register_rest_route(
REST_Routes::REST_ROOT,
'e2e/reference-url',
Expand Down
98 changes: 41 additions & 57 deletions tests/e2e/specs/api-cache.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,29 @@ import {
resetSiteKit,
safeLoginUser,
setupSiteKit,
testSiteNotification,
useRequestInterception,
wpApiFetch,
} from '../utils';
import { deleteAuthCookie } from '../utils/delete-auth-cookie';

const goToSiteKitDashboard = async () => {
await visitAdminPage( 'admin.php', 'page=googlesitekit-dashboard' );
};
async function goToWordPressDashboard() {
await visitAdminPage( 'index.php' );
}

async function googleSiteKitAPIGetTime() {
await page.waitForFunction(
() =>
window.googlesitekit !== undefined &&
window.googlesitekit.api !== undefined
);

return await page.evaluate( () => {
// `useCache` is enabled by default for `get` calls,
// but we'll be explicit here for the sake of the test.
return window.googlesitekit.api.get( 'e2e', 'util', 'time', null, {
useCache: true,
} );
} );
}

describe( 'API cache', () => {
beforeAll( async () => {
Expand All @@ -45,12 +59,6 @@ describe( 'API cache', () => {
const url = request.url();
if ( url.match( 'search-console/data/searchanalytics' ) ) {
request.respond( { status: 200, body: '[]' } );
} else if ( url.match( 'pagespeed-insights/data/pagespeed' ) ) {
request.respond( { status: 200, body: '{}' } );
} else if ( url.match( 'user/data/survey' ) ) {
request.respond( { status: 200, body: '{"survey":null}' } );
} else if ( url.match( 'user/data/survey-timeouts' ) ) {
request.respond( { status: 200, body: '[]' } );
} else {
request.continue();
}
Expand All @@ -64,60 +72,36 @@ describe( 'API cache', () => {
} );

it( 'isolates client storage between sessions', async () => {
const firstTestNotification = { ...testSiteNotification };
const secondTestNotification = {
...testSiteNotification,
id: 'test-notification-2',
title: 'Test notification title 2',
dismissLabel: 'test dismiss site notification 2',
};
// Navigate to WP dashboard to use SK APIs
// while minimizing side-effects from reporting, etc.
await goToWordPressDashboard();

// create first notification
await wpApiFetch( {
path: 'google-site-kit/v1/e2e/core/site/notifications',
method: 'post',
data: firstTestNotification,
const initialTimeData = await googleSiteKitAPIGetTime();
expect( initialTimeData ).toMatchObject( {
time: expect.any( Number ),
} );

await goToSiteKitDashboard();

await page.waitForSelector(
`#${ firstTestNotification.id }.googlesitekit-publisher-win--is-open`,
{ timeout: 10_000 } // Core site notifications are delayed 5s for surveys.
);

await expect( page ).toClick(
'.googlesitekit-publisher-win .mdc-button span',
{
text: firstTestNotification.dismissLabel,
}
);
// Show that the data is cached when fetching again.
const timeData = await googleSiteKitAPIGetTime();
expect( timeData ).toEqual( initialTimeData );

// create second notification
await wpApiFetch( {
path: 'google-site-kit/v1/e2e/core/site/notifications',
method: 'post',
data: secondTestNotification,
} );

// delete auth cookie to sign out the current user
// Delete auth cookie to sign out the current user.
// This was needed in the past due to cache clearing
// that was hooked into the logout action.
// Deleting cookies makes it more clear that the observed
// result is due to a new session only. It's also faster.
await deleteAuthCookie();

await safeLoginUser( 'admin', 'password' );

await goToSiteKitDashboard();

// Ensure the second notification is displayed.
await page.waitForSelector(
`#${ secondTestNotification.id }.googlesitekit-publisher-win--is-open`,
{ timeout: 10_000 } // Core site notifications are delayed 5s for surveys.
);
await goToWordPressDashboard();

await expect( page ).toMatchElement(
'.googlesitekit-publisher-win__title',
{
text: secondTestNotification.title,
}
);
// Now that we're in a new session, we expect the API cache to be clear
// so the request should hit the backend and produce a new (greater) value.
const newTimeData = await googleSiteKitAPIGetTime();
expect( initialTimeData ).toMatchObject( {
time: expect.any( Number ),
} );
expect( newTimeData.time ).toBeGreaterThan( initialTimeData.time );
} );
} );
3 changes: 2 additions & 1 deletion tests/phpunit/integration/Core/Modules/REST_Modules_ControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ public function test_activation_rest_endpoint__prevent_inactive_dependencies_act
// TODO: As Site Kit doesn't have any dependent modules at this moment,
// update this test case so that a dependency relationship can be
// mocked without referencing an actual module, e.g. using FakeModule.
$this->markTestSkipped( 'TODO' );
}

public function test_activation_rest_endpoint__activate_module() {
Expand Down Expand Up @@ -732,7 +733,7 @@ public function test_datapoint_rest_endpoint__post_invalid_slug() {
$this->controller->register();
$this->register_rest_routes();

$request = new WP_REST_Request( 'POST', '/' . REST_Routes::REST_ROOT . '/modules/non-existent-module/data/accounts-properties-profiles' );
$request = new WP_REST_Request( 'POST', '/' . REST_Routes::REST_ROOT . '/modules/non-existent-module/data/settings' );
$response = rest_get_server()->dispatch( $request );

$this->assertEquals( 'invalid_module_slug', $response->get_data()['code'] );
Expand Down

0 comments on commit 944cad1

Please sign in to comment.