test: fix join flow test flakiness

- wait for navigation to complete so we don't accidentally grab an
  element on the page we're navigating away from, leading to a stale
  reference
- wait for focus to move into a selected input field so we don't
  accidentally grab the `validation-message` attached to the old input
  box (the "username" box is selected by default on the join flow page)

test/integration/footer-links.test.js

@@ -5,7 +5,8 @@ const SeleniumHelper = require('./selenium-helpers.js');
 const {
     clickText,
     buildDriver,
-    findText
+    findText,
+    waitUntilDocumentReady
 } = new SeleniumHelper();
 
 const rootUrl = process.env.ROOT_URL || 'https://scratch.ly';
@@ -28,14 +29,9 @@ describe('www-integration footer links', () => {
 
     // ==== About Scratch column ====
 
-    const pageLoadComplete = () =>
-        driver.wait(async () => {
-            return await driver.executeScript('return document.readyState;') === 'complete';
-        });
-
     test('click About Scratch link', async () => {
         await clickText('About Scratch');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/about\/?$/);
@@ -43,7 +39,7 @@ describe('www-integration footer links', () => {
 
     test('click For Parents link', async () => {
         await clickText('For Parents');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/parents\/?$/);
@@ -51,7 +47,7 @@ describe('www-integration footer links', () => {
 
     test('click For Educators link', async () => {
         await clickText('For Educators');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/educators\/?$/);
@@ -59,7 +55,7 @@ describe('www-integration footer links', () => {
 
     test('click For Developers link', async () => {
         await clickText('For Developers');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/developers\/?$/);
@@ -69,7 +65,7 @@ describe('www-integration footer links', () => {
 
     test('click Community Guidelines link', async () => {
         await clickText('Community Guidelines');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/community_guidelines\/?$/);
@@ -77,7 +73,7 @@ describe('www-integration footer links', () => {
 
     test('click Discussion Forums link', async () => {
         await clickText('Discussion Forums');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/discuss\/?$/);
@@ -85,7 +81,7 @@ describe('www-integration footer links', () => {
 
     test('click Statistics link', async () => {
         await clickText('Statistics');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/statistics\/?$/);
@@ -95,7 +91,7 @@ describe('www-integration footer links', () => {
 
     test('click Ideas link', async () => {
         await clickText('Ideas');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/ideas\/?$/);
@@ -103,7 +99,7 @@ describe('www-integration footer links', () => {
 
     test('click FAQ link', async () => {
         await clickText('FAQ');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/faq\/?$/);
@@ -111,7 +107,7 @@ describe('www-integration footer links', () => {
 
     test('click Download link', async () => {
         await clickText('Download');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/download\/?$/);
@@ -119,7 +115,7 @@ describe('www-integration footer links', () => {
 
     test('click Contact Us link', async () => {
         await clickText('Contact Us');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/contact-us\/?$/);
@@ -129,7 +125,7 @@ describe('www-integration footer links', () => {
 
     test('click Terms of Use link', async () => {
         await clickText('Terms of Use');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/terms_of_use\/?$/);
@@ -137,7 +133,7 @@ describe('www-integration footer links', () => {
 
     test('click Privacy Policy link', async () => {
         await clickText('Privacy Policy');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/privacy_policy\/?$/);
@@ -145,7 +141,7 @@ describe('www-integration footer links', () => {
 
     test('click Cookies link', async () => {
         await clickText('Cookies');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/cookies\/?$/);
@@ -158,7 +154,7 @@ describe('www-integration footer links', () => {
 
     test('click DMCA link', async () => {
         await clickText('DMCA');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/DMCA\/?$/);
@@ -168,7 +164,7 @@ describe('www-integration footer links', () => {
 
     test('click Scratch Conference link', async () => {
         await clickText('Scratch Conference');
-        await pageLoadComplete();
+        await waitUntilDocumentReady();
         const url = await driver.getCurrentUrl();
         const pathname = (new URL(url)).pathname;
         expect(pathname).toMatch(/^\/scratch-conference\/?$/);

test/integration/join.test.js

@@ -3,9 +3,11 @@
 const SeleniumHelper = require('./selenium-helpers.js');
 
 const {
-    findByXpath,
+    buildDriver,
     clickXpath,
-    buildDriver
+    findByXpath,
+    navigate,
+    waitUntilDocumentReady
 } = new SeleniumHelper();
 
 const rootUrl = process.env.ROOT_URL || 'https://scratch.ly';
@@ -18,14 +20,14 @@ let driver;
 describe('www-integration join flow', () => {
     beforeAll(async () => {
         driver = await buildDriver('www-integration join flow');
-        await driver.get(rootUrl);
     });
 
     afterAll(() => driver.quit());
 
     beforeEach(async () => {
-        await driver.get(rootUrl);
-        await clickXpath('//a[@class="registrationLink"]');
+        await navigate(rootUrl); // navigate to home page
+        await clickXpath('//a[@class="registrationLink"]'); // navigate to join page
+        await waitUntilDocumentReady();
     });
 
     test('click Join opens join modal', async () => {
@@ -35,22 +37,24 @@ describe('www-integration join flow', () => {
     });
 
     test('username validation message appears', async () => {
-        await clickXpath('//input[contains(@name, "username")]');
+        const clickedInput = await clickXpath('//input[contains(@name, "username")]');
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput));
         const message = await findByXpath('//div[contains(@class, "validation-message")]');
         const messageText = await message.getText();
         expect(messageText).toEqual('Don\'t use your real name');
-
     });
 
     test('password validation message appears', async () => {
-        await clickXpath('//input[contains(@name, "password")]');
+        const clickedInput = await clickXpath('//input[contains(@name, "password")]');
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput));
         const message = await findByXpath('//div[contains(@class, "validation-message")]');
         const messageText = await message.getText();
         expect(messageText).toContain('Write it down so you remember.');
     });
 
     test('password confirmation validation message appears', async () => {
-        await clickXpath('//input[contains(@name, "passwordConfirm")]');
+        const clickedInput = await clickXpath('//input[contains(@name, "passwordConfirm")]');
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput));
         const message = await findByXpath('//div[contains(@class, "validation-message")]');
         const messageText = await message.getText();
         expect(messageText).toEqual('Type password again');
@@ -59,6 +63,7 @@ describe('www-integration join flow', () => {
     test('username validation: too short', async () => {
         const textInput = await findByXpath('//input[contains(@name, "username")]');
         await textInput.click();
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput));
         await textInput.sendKeys('ab');
         await clickXpath('//div[@class = "join-flow-outer-content"]');
         const message = await findByXpath('//div[contains(@class, "validation-error")]');
@@ -69,6 +74,7 @@ describe('www-integration join flow', () => {
     test('username validation: username taken', async () => {
         const textInput = await findByXpath('//input[contains(@name, "username")]');
         await textInput.click();
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput));
         await textInput.sendKeys(takenUsername);
         await clickXpath('//div[@class = "join-flow-outer-content"]');
         const message = await findByXpath('//div[contains(@class, "validation-error")]');
@@ -79,6 +85,7 @@ describe('www-integration join flow', () => {
     test('username validation: bad word', async () => {
         const textInput = await findByXpath('//input[contains(@name, "username")]');
         await textInput.click();
+        await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput));
         // Should be caught by the filter
         await textInput.sendKeys('xxxxxxxxx');
         await clickXpath('//div[@class = "join-flow-outer-content"]');

test/integration/selenium-helpers.js

@@ -136,8 +136,10 @@ class SeleniumHelper {
             'getLogs',
             'getSauceDriver',
             'isSignedIn',
+            'navigate',
             'signIn',
             'urlMatches',
+            'waitUntilDocumentReady',
             'waitUntilGone'
         ]);
 
@@ -248,6 +250,36 @@ class SeleniumHelper {
         return Key[keyName];
     }
 
+    /**
+     * Wait until the document is ready (i.e. the document.readyState is 'complete')
+     * @returns {Promise} A promise that resolves when the document is ready
+     */
+    async waitUntilDocumentReady () {
+        const outerError = new SeleniumHelperError('waitUntilDocumentReady failed');
+        try {
+            await this.driver.wait(async () =>
+                await this.driver.executeScript('return document.readyState;') === 'complete'
+            );
+        } catch (cause) {
+            throw await outerError.chain(cause, this.driver);
+        }
+    }
+
+    /**
+     * Navigate to the given URL and wait until the document is ready
+     * @param {string} url The URL to navigate to.
+     * @returns {Promise} A promise that resolves when the document is ready
+     */
+    async navigate (url) {
+        const outerError = new SeleniumHelperError('navigate failed', [{url}]);
+        try {
+            await this.driver.get(url);
+            await this.waitUntilDocumentReady();
+        } catch (cause) {
+            throw await outerError.chain(cause, this.driver);
+        }
+    }
+
     /**
      * Find an element by xpath.
      * @param {string} xpath The xpath to search for.
@@ -287,29 +319,33 @@ class SeleniumHelper {
         return this.driver.wait(async () => {
             const elementAtPath = await this.findByXpath(xpath);
             if (!elementAtPath) {
-                return;
+                return null;
             }
 
             if (allowScrolling) {
-                await this.driver.executeScript(
+                const info = await this.driver.executeScript(
                     `
+                    const info = {};
                     const element = arguments[0];
                     const boundingRect = element.getBoundingClientRect();
-                    boundingRect.windowWidth = window.innerWidth;
-                    boundingRect.windowHeight = window.innerHeight;
                     if (boundingRect.top < 0 || boundingRect.bottom > window.innerHeight ||
                         boundingRect.left < 0 || boundingRect.right > window.innerWidth)
                     {
-                        boundingRect.scrollIntoView = true;
                         element.scrollIntoView({
                             behavior: 'instant',
                             block:'nearest',
                             inline: 'nearest'
                         });
+                        info.didScroll = true;
                     }
+                    return info;
                     `,
                     elementAtPath
                 );
+                if (info.didScroll) {
+                    // try again after the scroll completes
+                    return null;
+                }
             }
 
             const elementAtPoint = await this.driver.executeScript(
@@ -323,7 +359,7 @@ class SeleniumHelper {
                 elementAtPath
             );
             if (!elementAtPoint) {
-                return;
+                return null;
             }
             // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK.
             // It doesn't have to be an exact match.
@@ -333,13 +369,13 @@ class SeleniumHelper {
                 elementAtPoint
             );
             if (!match) {
-                return;
+                return null;
             }
             if (!await elementAtPath.isDisplayed()) {
-                return;
+                return null;
             }
             if (!await elementAtPath.isEnabled()) {
-                return;
+                return null;
             }
             return elementAtPath;
         });
@@ -348,13 +384,15 @@ class SeleniumHelper {
     /**
      * Wait until an element can be found by the provided xpath, then click on it.
      * @param {string} xpath The xpath to click.
+     * @param {boolean} [allowScrolling] Whether or not to allow page scrolling to reach the element.
      * @returns {Promise} A promise that resolves when the element is clicked.
      */
-    async clickXpath (xpath) {
+    async clickXpath (xpath, allowScrolling = true) {
         const outerError = new SeleniumHelperError('clickXpath failed', [{xpath}]);
         try {
-            const element = await this.waitUntilClickable(xpath);
+            const element = await this.waitUntilClickable(xpath, allowScrolling);
             element.click();
+            return element;
         } catch (cause) {
             throw await outerError.chain(cause, this.driver);
         }