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

test(functional): Convert Robots test, oauth webchannel tests #15379

Merged
merged 1 commit into from
Jun 15, 2023
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
22 changes: 22 additions & 0 deletions packages/functional-tests/lib/channels.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

export enum FirefoxCommand {
FxAStatus = 'fxaccounts:fxa_status',
OAuthLogin = 'fxaccounts:oauth_login',
Logout = 'fxaccounts:logout',
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 tried using this in a couple other tests and ran into some type issues with createCustomEventDetail so it might need tweaking but I think this works for now.

We may want to move these from settings into a shared file.

}

export function createCustomEventDetail(
command: FirefoxCommand,
data: Record<string, any>
) {
return {
id: 'account_updates',
message: {
command,
data,
},
};
}
54 changes: 54 additions & 0 deletions packages/functional-tests/pages/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,58 @@ export abstract class BaseLayout {
addEventListener('WebChannelMessageToChrome', listener);
});
}

/**
* Listens for a `WebChannelMessageToChrome` web channel event which
* occurs when we (web content) send a message to the browser.
*
* Responds with a `WebChannelMessageToContent` event containing event
* details passed in only when the given command matches the command from
* the listened-for event.
*
* @param webChannelMessage - Custom event details to send to the web content.
*/
async respondToWebChannelMessage(webChannelMessage) {
const expectedCommand = webChannelMessage.message.command;
const response = webChannelMessage.message.data;

await this.page.evaluate(
({ expectedCommand, response }) => {
function listener(e) {
const detail = JSON.parse((e as CustomEvent).detail);
const command = detail.message.command;
const messageId = detail.message.messageId;

if (command === expectedCommand) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vbudhram I was able to remove the attached div stuff and it passes every time for me, it felt OK to remove. I can also remove some of this if (command === expectedCommand) { logic and it works, but, I'm not sure we should strip that out yet because I think we may need it in other tests if we need to wait for some event that doesn't fire immediately.

window.removeEventListener('WebChannelMessageToChrome', listener);
const event = new CustomEvent('WebChannelMessageToContent', {
detail: {
id: 'account_updates',
message: {
command,
data: response,
messageId,
},
},
});

window.dispatchEvent(event);
}
}

function startListening() {
try {
window.addEventListener('WebChannelMessageToChrome', listener);
} catch (e) {
// problem adding the listener, window may not be
// ready, try again.
setTimeout(startListening, 0);
}
}

startListening();
},
{ expectedCommand, response }
);
}
}
61 changes: 51 additions & 10 deletions packages/functional-tests/pages/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ export const selectors = {
NOT_EMAIL_MET: '#password-same-as-email.password-strength-met',
NOT_EMAIL_FAIL: '#password-same-as-email.password-strength-fail',
PERMISSION_ACCEPT: '#accept',
CWTS_HEADER: 'text="Choose what to sync"',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we start using text selectors here where we can, it's less work when we get to the React conversion work.

CWTS_ENGINE_BOOKMARKS: '#sync-engine-bookmarks',
CWTS_ENGINE_HISTORY: '#sync-engine-history',
};

type FirstSignUpOptions = {
verify?: boolean;
enterEmail?: boolean;
waitForNavOnSubmit?: boolean;
};

export class LoginPage extends BaseLayout {
Expand Down Expand Up @@ -87,7 +96,12 @@ export class LoginPage extends BaseLayout {
await this.submit();
}

async login(email: string, password: string, recoveryCode?: string) {
async login(
email: string,
password: string,
recoveryCode?: string,
waitForNavOnSubmit = true
) {
// When running tests in parallel, playwright shares the storage state,
// so we might not always be at the email first screen.
if (await this.isCachedLogin()) {
Expand All @@ -106,7 +120,7 @@ export class LoginPage extends BaseLayout {
await this.setPassword(password);
}

await this.submit();
await this.submit(waitForNavOnSubmit);
if (recoveryCode) {
await this.clickUseRecoveryCode();
await this.setCode(recoveryCode);
Expand Down Expand Up @@ -165,8 +179,11 @@ export class LoginPage extends BaseLayout {
async fillOutFirstSignUp(
email: string,
password: string,
verify = true,
enterEmail = true
{
verify = true,
enterEmail = true,
waitForNavOnSubmit = true,
}: FirstSignUpOptions = {}
) {
if (enterEmail) {
await this.setEmail(email);
Expand All @@ -177,18 +194,18 @@ export class LoginPage extends BaseLayout {
await this.page.fill(selectors.AGE, '24');
await this.submit();
if (verify) {
await this.fillOutSignUpCode(email);
await this.fillOutSignUpCode(email, waitForNavOnSubmit);
}
}

async fillOutSignUpCode(email: string) {
async fillOutSignUpCode(email: string, waitForNavOnSubmit = true) {
const code = await this.target.email.waitForEmail(
email,
EmailType.verifyShortCode,
EmailHeader.shortCode
);
await this.setCode(code);
await this.submit();
await this.submit(waitForNavOnSubmit);
}

async fillOutSignInCode(email: string) {
Expand Down Expand Up @@ -319,10 +336,16 @@ export class LoginPage extends BaseLayout {
await this.submit();
}

async submit() {
const waitForNavigation = this.page.waitForNavigation();
async submit(waitForNav = true) {
if (waitForNav) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be able to use waitForUrl and have it accept a RegEx of possible paths? I haven't seen these waitForNavigation errors pop up in a while 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.

Looks like we should be able to and that it's the recommendation now:

image

I could change this for the one test, but I think that might be better in a follow up since we'd still need some conditional here until we do it across the board for all tests that use this - it looks like we call this.submit in login.ts for various things like setTotp and setNewPassword etc. so we need to pass the expected URL regex in there when we call this.submit.

const waitForNavigation = this.page.waitForNavigation();
await this.page.locator(selectors.SUBMIT).click();
return waitForNavigation;
}
// `waitForNavigation` is deprecated because it's "hard to make it work reliably",
// see https://github.com/microsoft/playwright/issues/20853. It's causing at least
// one set of test failures so give an option to opt-out here.
await this.page.locator(selectors.SUBMIT).click();
return waitForNavigation;
}

async clickForgotPassword() {
Expand Down Expand Up @@ -355,6 +378,24 @@ export class LoginPage extends BaseLayout {
return this.page.locator(selectors.SUBMIT).click();
}

async waitForCWTSHeader() {
await this.page.waitForSelector(selectors.CWTS_HEADER, {
Copy link
Contributor Author

@LZoog LZoog Jun 9, 2023

Choose a reason for hiding this comment

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

I don't think our other tests with this...

    return this.page.isVisible(selectors.SYNC_CONNECTED_HEADER, {
      timeout: 100,
    });

... actually wait. It looks like isVisible executes immediately, even if I slapped a 5000 second timeout it wouldn't wait. Personally I think simply using this.page.waitForSelector in our selector checks and then a simple await waitForCWTSHeader(); call is nice over checking for truthiness anyway; if it doesn't find the element the test fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm onboard to make this change 👍🏽

timeout: 100,
});
}

async waitForCWTSEngineBookmarks() {
await this.page.waitForSelector(selectors.CWTS_ENGINE_BOOKMARKS, {
timeout: 100,
});
}

async waitForCWTSEngineHistory() {
await this.page.waitForSelector(selectors.CWTS_ENGINE_HISTORY, {
timeout: 100,
});
}

async isSyncConnectedHeader() {
return this.page.isVisible(selectors.SYNC_CONNECTED_HEADER, {
timeout: 100,
Expand Down
10 changes: 10 additions & 0 deletions packages/functional-tests/tests/misc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ test.describe('severity-1', () => {
expect(await relier.isLoggedIn()).toBe(true);
});
});

test.describe('robots.txt', () => {
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'd written this one a while ago, I can remove if we no longer want this test but we don't have any unit tests for it so it seems good to leave to me?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can leave it on here!

test('should allow bots to access all pages', async ({ target, page }) => {
await page.goto(`${target.contentServerUrl}/robots.txt`, {
waitUntil: 'networkidle',
Copy link
Contributor

Choose a reason for hiding this comment

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

From latest docs 😅 -networkidle - **DISCOURAGED** consider operation to be finished when there are no network connections for at least 500 ms. Don't use this method for testing, rely on web assertions to assess readiness instead.

Not for this PR but good for follow up.

});
const text = await page.locator('body').innerText();
expect(/^Allow:/gm.test(text)).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test.describe('oauth permissions for trusted reliers', () => {
}) => {
await relier.goto();
await relier.clickEmailFirst();
await login.fillOutFirstSignUp(email, password, false);
await login.fillOutFirstSignUp(email, password, { verify: false });

//no permissions asked for, straight to confirm
expect(await login.isSignUpCodeHeader()).toBe(true);
Expand All @@ -50,7 +50,7 @@ test.describe('oauth permissions for trusted reliers', () => {
waitUntil: 'networkidle',
});
await relier.clickEmailFirst();
await login.fillOutFirstSignUp(email, password, false);
await login.fillOutFirstSignUp(email, password, { verify: false });

//Verify permissions header
expect(await login.permissionsHeader()).toBe(true);
Expand Down
6 changes: 3 additions & 3 deletions packages/functional-tests/tests/oauth/signUp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ test.describe('Oauth sign up', () => {
test('sign up', async ({ pages: { login, relier } }) => {
await relier.goto();
await relier.clickEmailFirst();
await login.fillOutFirstSignUp(email, password, false);
await login.fillOutFirstSignUp(email, password, { verify: false });

//Verify sign up code header
expect(await login.isSignUpCodeHeader()).toBe(true);
Expand All @@ -44,7 +44,7 @@ test.describe('Oauth sign up', () => {

await relier.goto();
await relier.clickEmailFirst();
await login.fillOutFirstSignUp(bouncedEmail, password, false);
await login.fillOutFirstSignUp(bouncedEmail, password, { verify: false });

//Verify sign up code header
expect(await login.isSignUpCodeHeader()).toBe(true);
Expand All @@ -56,7 +56,7 @@ test.describe('Oauth sign up', () => {
);

await login.clearEmailTextBox();
await login.fillOutFirstSignUp(email, password, false);
await login.fillOutFirstSignUp(email, password, { verify: false });

//Verify sign up code header
expect(await login.isSignUpCodeHeader()).toBe(true);
Expand Down
4 changes: 2 additions & 2 deletions packages/functional-tests/tests/oauth/signin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ test.describe('OAuth signin', () => {
await relier.clickEmailFirst();

// Dont register account and attempt to login via relier
await login.fillOutFirstSignUp(email, password, false);
await login.fillOutFirstSignUp(email, password, { verify: false });

await relier.goto();
await relier.clickEmailFirst();
Expand Down Expand Up @@ -127,7 +127,7 @@ test.describe('OAuth signin', () => {

await relier.goto();
await relier.clickChooseFlow();
await login.fillOutFirstSignUp(email, PASSWORD, false);
await login.fillOutFirstSignUp(email, PASSWORD, { verify: false });

// go back to the OAuth app, the /oauth flow should
// now suggest a cached login
Expand Down
4 changes: 2 additions & 2 deletions packages/functional-tests/tests/oauth/syncSignIn.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test.describe('signin with OAuth after Sync', () => {
await relier.goto();
await relier.clickEmailFirst();
await login.useDifferentAccountLink();
await login.fillOutFirstSignUp(email2, password, true);
await login.fillOutFirstSignUp(email2, password);

// RP is logged in, logout then back in again
expect(await relier.isLoggedIn()).toBe(true);
Expand Down Expand Up @@ -66,7 +66,7 @@ test.describe('signin to Sync after OAuth', () => {
const email = login.createEmail('sync{id}');
await relier.goto();
await relier.clickEmailFirst();
await login.fillOutFirstSignUp(email, password, true);
await login.fillOutFirstSignUp(email, password);
expect(await relier.isLoggedIn()).toBe(true);
await page.goto(
`${target.contentServerUrl}?context=fx_desktop_v3&service=sync&action=email&`
Expand Down
73 changes: 73 additions & 0 deletions packages/functional-tests/tests/oauth/webchannel.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { FirefoxCommand, createCustomEventDetail } from '../../lib/channels';
import { test, expect } from '../../lib/fixtures/standard';

const PASSWORD = 'Password123!';

test.describe('oauth webchannel', () => {
test.beforeEach(async ({ pages: { login } }) => {
await login.clearCache();
});

test('signup', async ({ pages: { login, relier } }) => {
const customEventDetail = createCustomEventDetail(
FirefoxCommand.FxAStatus,
{
capabilities: {
choose_what_to_sync: true,
engines: ['bookmarks', 'history'],
},
signedInUser: null,
}
);
const email = login.createEmail();

await relier.goto('context=oauth_webchannel_v1&automatedBrowser=true');
await relier.clickEmailFirst();

await login.respondToWebChannelMessage(customEventDetail);

await login.setEmail(email);
await login.submit();

// the CWTS form is on the same signup page
await login.waitForCWTSHeader();
await login.waitForCWTSEngineBookmarks();
await login.waitForCWTSEngineHistory();

await login.fillOutFirstSignUp(email, PASSWORD, {
enterEmail: false,
waitForNavOnSubmit: false,
});
await login.checkWebChannelMessage(FirefoxCommand.OAuthLogin);
});

test('signin', async ({ pages: { login, relier, page }, credentials }) => {
const customEventDetail = createCustomEventDetail(
FirefoxCommand.FxAStatus,
{
capabilities: {
engines: ['bookmarks', 'history'],
},
signedInUser: null,
}
);

await relier.goto('context=oauth_webchannel_v1&automatedBrowser=true');
await relier.clickEmailFirst();

await login.respondToWebChannelMessage(customEventDetail);

const { searchParams } = new URL(page.url());
expect(searchParams.has('client_id')).toBe(true);
expect(searchParams.has('redirect_uri')).toBe(true);
expect(searchParams.has('state')).toBe(true);
expect(searchParams.has('context')).toBe(true);

await login.login(credentials.email, credentials.password, '', false);
await login.checkWebChannelMessage('fxaccounts:oauth_login');
});
});
Loading