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.

Sign up for GitHub

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

Jump to bottom

Flaky Jetpack E2E: revise Advertising: Promote spec to use the first post and to not create posts unnecessarily. #81563

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Flaky Jetpack E2E: revise Advertising: Promote spec to use the first post and to not create posts unnecessarily. #81563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19b9206
packages/calypso-e2e/src/rest-api-client.ts
worldomonation Sep 8, 2023
c597382
packages/calypso-e2e/src/rest-api-client.ts
worldomonation Sep 11, 2023
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
65 changes: 30 additions & 35 deletions packages/calypso-e2e/src/rest-api-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
PublicizeConnectionDeletedResponse,
PublicizeConnection,
SubscriberDeletedResponse,
PostCountsResponse,
} from './types';
import type { Roles } from './lib';
import type {
Expand Down Expand Up @@ -42,6 +43,7 @@ import type {
JetpackSearchResponse,
JetpackSearchParams,
Subscriber,
SitePostState,
} from './types';
import type { BodyInit, HeadersInit, RequestInit } from 'node-fetch';

Expand Down Expand Up @@ -79,7 +81,6 @@ export class RestAPIClient {

/**
* Constructs an instance of the API client.
*
* @param {AccountCredentials} credentials User credentials.
* @param {string} [bearerToken] BearerToken for the user.
*/
Expand All @@ -93,7 +94,6 @@ export class RestAPIClient {
*
* If the token has been previously obtained, this method returns the value.
* Otherwise, an API call is made to obtain the bearer token and the resulting value is returned
*
* @returns {Promise<string>} String representing the bearer token.
* @throws {Error} If the API responded with a success status of false.
*/
Expand Down Expand Up @@ -132,7 +132,6 @@ export class RestAPIClient {

/**
* Returns the appropriate authorization header.
*
* @returns {Promise<string>} Authorization header in the requested scheme.
* @throws {Error} If a scheme not yet implemented is requested.
*/
Expand All @@ -147,7 +146,6 @@ export class RestAPIClient {

/**
* Returns the formatted Content-Type header string.
*
* @returns {string} Content-Type header string.
*/
private getContentTypeHeader( value: 'json' ): string {
Expand All @@ -156,7 +154,6 @@ export class RestAPIClient {

/**
* Returns a fully constructed URL object pointing to the request endpoint.
*
* @param {EndpointVersions} version Version of the API to use.
* @param {string} endpoint REST API path.
* @param {EndpointNamespace} [namespace] REST API namespace.
Expand All @@ -174,7 +171,6 @@ export class RestAPIClient {

/**
* Sends the request to the endpoint, then returns the decoded JSON.
*
* @param {URL} url URL of the endpoint.
* @param {RequestParams} params Parameters for the request.
* @returns {Promise<any>} Decoded JSON response.
Expand All @@ -192,9 +188,8 @@ export class RestAPIClient {
* This method returns an array of DomainData objects, where
* each object exposes a few key pieces of data from
* the response JSON:
* - domain
* - blog id
*
* - domain
* - blog id
* @returns {Promise<AllDomainsResponse>} JSON array of sites.
* @throws {Error} If API responded with an error.
*/
Expand All @@ -220,7 +215,6 @@ export class RestAPIClient {

/**
* Given parameters, create a new site.
*
* @param {NewSiteParams} newSiteParams Details for the new site.
* @returns {Promise<NewSiteResponse>} Confirmation details for the new site.
* @throws {ErrorResponse} If API responded with an error.
Expand Down Expand Up @@ -268,7 +262,6 @@ export class RestAPIClient {
*
* Otherwise the active subscription must be first cancelled
* or else the REST API will throw a HTTP 403 status.
*
* @param { {id: number, domain: string}} targetSite Details for the target site to be deleted.
* @returns {SiteDeletionResponse | null} Null if deletion was unsuccessful or not performed. SiteDeletionResponse otherwise.
*/
Expand Down Expand Up @@ -322,7 +315,6 @@ export class RestAPIClient {

/**
* Creates a user invite.
*
* @param {number} siteID ID of the site where a new invite will be created.
* @param param0 Keyed object parameter.
* @param {string[]} param0.email List of emails to send invites to.
Expand Down Expand Up @@ -463,7 +455,6 @@ export class RestAPIClient {
/**
* Returns the account information for the user authenticated
* via the bearer token.
*
* @returns {Promise<MyAccountInformationResponse>} Response containing user details.
* @throws {Error} If API responded with an error.
*/
Expand All @@ -489,7 +480,6 @@ export class RestAPIClient {

/**
* Updates the user's settings.
*
* @param {SettingsParams} details Key/value attributes to be set for the user.
* @returns { { [key: string]: string | number } } Generic object.
* @throws {Error} If an unknown attribute or invalid value for a known attribute was provided.
Expand Down Expand Up @@ -525,7 +515,6 @@ export class RestAPIClient {
* The userID, username and email of the account that is
* authenticated via the bearer token is checked against the
* supplied parameters.
*
* @param { AccountDetails} expectedAccountDetails Details of the accounts to be closed.
* @returns {Promise<boolean>} True if account closure was successful. False otherwise.
*/
Expand Down Expand Up @@ -587,7 +576,6 @@ export class RestAPIClient {

/**
* Returns Calypso preferences for the user.
*
* @returns {Promise<CalypsoPreferencesResponse>} JSON response containing Calypso preferences.
*/
async getCalypsoPreferences(): Promise< CalypsoPreferencesResponse > {
Expand All @@ -606,7 +594,6 @@ export class RestAPIClient {

/**
* Gets the latest posts from blogs a user follows.
*
* @returns {Promise<ReaderResponse>} An Array of posts.
* @throws {Error} If API responded with an error.
*/
Expand Down Expand Up @@ -635,9 +622,34 @@ export class RestAPIClient {

/* Posts */

/**
* Given a siteID, checks whether any posts exists of a given state.
* @param {number} siteID Site ID.
* @param param1 Keyed object parameter.
* @param {SitePostState} param1.state State of the published post.
*/
async siteHasPost(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): We can also check for the expected response structure & API-specific errors.

if ( response.hasOwnProperty( 'error' ) ) {
	throw new Error(
		`${ ( response as ErrorResponse ).error }: ${ ( response as ErrorResponse ).message }`
	);
}

if (!response || !response.counts || !response.counts.all) {
	throw new Error('Unexpected response structure');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I played around with this a bit, and it appears this endpoint doesn't ever return an error - it only returns a 404 (if anything in the path is wrong, for example site ID) or the actual values.

siteID: number,
{ state = 'publish' }: { state: SitePostState }
): Promise< boolean > {
const params: RequestParams = {
method: 'get',
headers: {
Authorization: await this.getAuthorizationHeader( 'bearer' ),
'Content-Type': this.getContentTypeHeader( 'json' ),
},
};

const response: PostCountsResponse = await this.sendRequest(
this.getRequestURL( '1.1', `/sites/${ siteID }/post-counts/post` ),
params
);

return response.counts.all[ state ] !== 0;
}

/**
* Creates a post on the site.
*
* @param {number} siteID Target site ID.
* @param {NewPostParams} details Details of the new post.
*/
Expand Down Expand Up @@ -667,7 +679,6 @@ export class RestAPIClient {

/**
* Deletes a post denoted by postID from the site.
*
* @param {number} siteID Target site ID.
* @param {number} postID Target post ID.
*/
Expand Down Expand Up @@ -698,7 +709,6 @@ export class RestAPIClient {

/**
* Creates a comment on the given post.
*
* @param {number} siteID Target site ID.
* @param {number} postID Target post ID.
* @param {string} comment Details of the new comment.
Expand Down Expand Up @@ -735,7 +745,6 @@ export class RestAPIClient {

/**
* Deletes a given comment from a site.
*
* @param {number} siteID Target site ID.
* @param {number} commentID Target comment ID.
* @returns {Promise< any >} Decoded JSON response.
Expand Down Expand Up @@ -766,7 +775,6 @@ export class RestAPIClient {

/**
* Method to perform two similar operations - like and unlike a comment.
*
* @param {'like'|'unlike'} action Action to perform on the comment.
* @param {number} siteID Target site ID.
* @param {number} commentID Target comment ID.
Expand Down Expand Up @@ -818,7 +826,6 @@ export class RestAPIClient {

/**
* Uploads a media file.
*
* @param {number} siteID Target site ID.
* @param param1 Optional object parameter.
* @param {TestFile} param1.media Local media file to be uploaded.
Expand Down Expand Up @@ -878,7 +885,6 @@ export class RestAPIClient {

/**
* Clears the shopping cart.
*
* @param {number} siteID Site that has the shopping cart.
* @throws {Error} If the user doesn't have access to the siteID.
* @returns {{success:true}} If the request was successful.
Expand Down Expand Up @@ -910,7 +916,6 @@ export class RestAPIClient {

/**
* Gets a list of plugins installed in a site.
*
* @param {number} siteID Target site ID.
* @returns {Promise<AllPluginsResponse>} An Array of plugins.
* @throws {Error} If API responded with an error.
Expand Down Expand Up @@ -940,7 +945,6 @@ export class RestAPIClient {

/**
* Modifies a plugin installed in a site.
*
* @param {number} siteID Target site ID.
* @param {string} pluginID Plugin ID.
* @param {PluginResponse} details Key/value attributes to be set for the user.
Expand Down Expand Up @@ -977,7 +981,6 @@ export class RestAPIClient {

/**
* Finds a plugin by name, deactivates it, and removes it from the site.
*
* @returns {Promise<PluginRemovalResponse | null>} Null if plugin removal was unsuccessful or not performed. PluginRemovalResponse otherwise.
* @throws {Error} If API responded with an error.
*/
Expand Down Expand Up @@ -1019,7 +1022,6 @@ export class RestAPIClient {
*
* As noted in the comments, this method is quite overloaded as its outcome
* differs depending on the current state of the widget (activate/deactivated).
*
* @param {number} siteID ID of the target site.
* @param {string} widgetID ID of the target widget.
*/
Expand Down Expand Up @@ -1059,7 +1061,6 @@ export class RestAPIClient {

/**
* Returns the list of widgets for a siteID.
*
* @param {number} siteID ID of the target site.
* @returns {AllWidgetsResponse} Array of Widgets object describing the list of widgets on the site.
*/
Expand Down Expand Up @@ -1088,7 +1089,6 @@ export class RestAPIClient {

/**
* Deletes or deactivates all widgets for a given site.
*
* @param {number} siteID ID of the target site.
*/
async deleteAllWidgets( siteID: number ): Promise< void > {
Expand All @@ -1102,7 +1102,6 @@ export class RestAPIClient {
/**
* Execute a primitive Jetpack site search request.
* Useful for checking if something has been indexed yet.
*
* @param {number} siteId ID of the target site.
* @param {JetpackSearchParams} searchParams The search parameters.
*/
Expand Down Expand Up @@ -1141,7 +1140,6 @@ export class RestAPIClient {

/**
* Returns an array of existing publicize (social) connections.
*
* @param {number} siteID Site ID.
* @returns {Promise<Array<PublicizeConnection>>} Array of Publicize connections.
*/
Expand Down Expand Up @@ -1170,7 +1168,6 @@ export class RestAPIClient {

/**
* Given siteID and connectionID, deletes the connection.
*
* @param {number} siteID Site ID.
* @param {number} connectionID Publicize connection ID.
* @returns {Promise<PublicizeConnectionDeletedResponse>} Confirmation of connection being deleted.
Expand Down Expand Up @@ -1200,7 +1197,6 @@ export class RestAPIClient {

/**
* Given a site ID, returns the list of newsletter subscribers.
*
* @param {number} siteID Site ID to return list of users for.
*/
async getAllSubscribers( siteID: number ): Promise< Subscriber[] > {
Expand All @@ -1224,7 +1220,6 @@ export class RestAPIClient {
/**
* Given a siteID and email address of the subscribed user to delete,
* removes the subscribed user.
*
* @param {number} siteID Site ID where the user is subscribed.
* @param {string} email Email address of the subscriber to delete.
*/
Expand Down
18 changes: 18 additions & 0 deletions packages/calypso-e2e/src/types/rest-api-client.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* Parameter Interfaces */

export type SitePostState = 'publish' | 'draft' | 'trash' | 'future';

export interface AccountDetails {
userID: number;
username: string;
Expand Down Expand Up @@ -223,6 +225,22 @@ export interface SubscriberDeletedResponse {
deleted: true;
}

export interface PostCountsResponse {
counts: {
all: {
publish: number;
future: number;
draft: number;
trash: number;
};
mine: {
draft: number;
publish: number;
trash: number;
};
};
}

/* Error Responses */

export interface BearerTokenErrorResponse {
Expand Down
26 changes: 19 additions & 7 deletions test/e2e/specs/tools/advertising__promote.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,23 @@ skipDescribeIf( envVariables.ATOMIC_VARIATION === 'private' )(
const accountName = getTestAccountByFeature( envToFeatureKey( envVariables ) );
testAccount = new TestAccount( accountName );

// Createa a new test post before starting the test, to ensure at least one
// available post.
restAPIClient = new RestAPIClient( testAccount.credentials );
newPostDetails = await restAPIClient.createPost(

// Createa a new test post before starting the test if site has no published post.
const hasPosts = await restAPIClient.siteHasPost(
testAccount.credentials.testSites?.primary.id as number,
{
title: pageTitle,
}
{ state: 'publish' }
);

if ( ! hasPosts ) {
newPostDetails = await restAPIClient.createPost(
testAccount.credentials.testSites?.primary.id as number,
{
title: pageTitle,
}
);
}

await testAccount.authenticate( page );

advertisingPage = new AdvertisingPage( page );
Expand All @@ -75,11 +82,12 @@ skipDescribeIf( envVariables.ATOMIC_VARIATION === 'private' )(
} );

it( 'Click on Promote for the first post', async function () {
await advertisingPage.clickButtonByNameOnRow( 'Promote', { postTitle: pageTitle } );
await advertisingPage.clickButtonByNameOnRow( 'Promote', { row: 1 } );
} );

it( 'Land in Blaze campaign landing page', async function () {
await page.waitForURL( /advertising/ );

blazeCampaignPage = new BlazeCampaignPage( page );
} );

Expand All @@ -102,6 +110,10 @@ skipDescribeIf( envVariables.ATOMIC_VARIATION === 'private' )(
} );

afterAll( async function () {
if ( ! newPostDetails ) {
return;
}

await restAPIClient.deletePost(
testAccount.credentials.testSites?.primary.id as number,
newPostDetails.ID
Expand Down
Loading