fix: Improve node ID check robustness & WebdriverIO test CI stability

.github/workflows/test.yml

@@ -63,7 +63,7 @@ jobs:
       - name: Run tests
         run: |
           cd main
-          npm run test
+          npm run test:ci
 
   webdriverio_tests:
     name: WebdriverIO tests (against pinned v12)
@@ -90,4 +90,4 @@ jobs:
         run: npm install
 
       - name: Run tests
-        run: npm run test
+        run: npm run test:ci

package.json

@@ -15,13 +15,16 @@
     "prepublishOnly": "npm login --registry https://wombat-dressing-room.appspot.com",
     "start": "blockly-scripts start",
     "test": "npm run test:mocha && npm run test:wdio",
+    "test:ci": "npm run test:mocha && npm run test:wdio:ci",
     "test:mocha": "blockly-scripts test",
     "test:wdio": "npm run wdio:clean && npm run wdio:run",
+    "test:wdio:ci": "npm run wdio:clean && npm run wdio:run:ci",
     "wdio:build": "npm run wdio:build:app && npm run wdio:build:tests",
     "wdio:build:app": "cd test/webdriverio && webpack",
     "wdio:build:tests": "tsc -p ./test/webdriverio/test/tsconfig.json",
     "wdio:clean": "cd test/webdriverio/test && rm -rf dist",
-    "wdio:run": "npm run wdio:build && cd test/webdriverio/test && npx mocha dist"
+    "wdio:run": "npm run wdio:build && cd test/webdriverio/test && npx mocha dist",
+    "wdio:run:ci": "npm run wdio:build && cd test/webdriverio/test && npx mocha --timeout 30000 dist"
   },
   "main": "./dist/index.js",
   "module": "./src/index.js",

test/webdriverio/test/actions_test.ts

@@ -92,12 +92,10 @@ suite('Menus test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    // This is the first test suite, which must wait for Chrome +
-    // chromedriver to start up, which can be slow—perhaps a few
-    // seconds.  Allow 30s just in case.
-    this.timeout(30000);
-
-    this.browser = await testSetup(testFileLocations.MORE_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.MORE_BLOCKS,
+      this.timeout(),
+    );
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/basic_test.ts

@@ -31,7 +31,10 @@ suite('Keyboard navigation on Blocks', function () {
 
   // Clear the workspace and load start blocks.
   suiteSetup(async function () {
-    this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.NAVIGATION_TEST_BLOCKS,
+      this.timeout(),
+    );
   });
 
   test('Default workspace', async function () {
@@ -226,7 +229,10 @@ suite('Keyboard navigation on Fields', function () {
 
   // Clear the workspace and load start blocks.
   suiteSetup(async function () {
-    this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.NAVIGATION_TEST_BLOCKS,
+      this.timeout(),
+    );
   });
 
   test('Up from first field selects block', async function () {

test/webdriverio/test/block_comment_test.ts

@@ -23,7 +23,10 @@ suite('Block comment navigation', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.NAVIGATION_TEST_BLOCKS,
+      this.timeout(),
+    );
     await this.browser.execute(() => {
       Blockly.getMainWorkspace()
         .getBlockById('p5_canvas_1')
@@ -36,7 +39,8 @@ suite('Block comment navigation', function () {
     await keyRight(this.browser);
     await sendKeyAndWait(this.browser, Key.Enter);
     const focusedNodeId = await getCurrentFocusNodeId(this.browser);
-    chai.assert.equal(focusedNodeId, 'blockly-2s_comment_textarea_');
+    chai.assert.isDefined(focusedNodeId);
+    chai.assert.match(focusedNodeId, /blockly-\w+_comment_textarea_/);
   });
 
   test('Escape from a focused comment focuses its bubble', async function () {

test/webdriverio/test/clipboard_test.ts

@@ -27,7 +27,7 @@ suite('Clipboard test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/delete_test.ts

@@ -27,7 +27,10 @@ suite('Deleting Blocks', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.NAVIGATION_TEST_BLOCKS,
+      this.timeout(),
+    );
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/duplicate_test.ts

@@ -23,7 +23,7 @@ suite('Duplicate test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/flyout_test.ts

@@ -28,7 +28,7 @@ suite('Toolbox and flyout test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/hooks.ts

@@ -16,7 +16,7 @@ export const mochaHooks: RootHookObject = {
   async beforeAll(this: Mocha.Context) {
     // Set a long timeout for startup.
     this.timeout(60000);
-    return await driverSetup();
+    return await driverSetup(this.timeout());
   },
   async afterAll() {
     return await driverTeardown();

test/webdriverio/test/insert_test.ts

@@ -28,7 +28,7 @@ suite('Insert test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/keyboard_mode_test.ts

@@ -32,7 +32,10 @@ suite(
 
     setup(async function () {
       // Reload the page between tests
-      this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+      this.browser = await testSetup(
+        testFileLocations.NAVIGATION_TEST_BLOCKS,
+        this.timeout(),
+      );
 
       await this.browser.pause(PAUSE_TIME);
 

test/webdriverio/test/move_test.ts

@@ -25,7 +25,10 @@ suite('Move start tests', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.MOVE_START_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.MOVE_START_TEST_BLOCKS,
+      this.timeout(),
+    );
     await this.browser.pause(PAUSE_TIME);
   });
 
@@ -186,6 +189,7 @@ suite('Statement move tests', function () {
   setup(async function () {
     this.browser = await testSetup(
       testFileLocations.MOVE_STATEMENT_TEST_BLOCKS,
+      this.timeout(),
     );
     await this.browser.pause(PAUSE_TIME);
   });
@@ -468,6 +472,7 @@ suite(`Value expression move tests`, function () {
           createTestUrl(
             new URLSearchParams({renderer, scenario: 'moveValueTestBlocks'}),
           ),
+          this.timeout(),
         );
         await this.browser.pause(PAUSE_TIME);
       });

test/webdriverio/test/mutator_test.ts

@@ -26,7 +26,10 @@ suite('Mutator navigation', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS);
+    this.browser = await testSetup(
+      testFileLocations.NAVIGATION_TEST_BLOCKS,
+      this.timeout(),
+    );
     this.openMutator = async () => {
       await focusOnBlock(this.browser, 'controls_if_1');
       await this.browser.pause(PAUSE_TIME);

test/webdriverio/test/scroll_test.ts

@@ -25,7 +25,7 @@ suite('Scrolling into view', function () {
   //
   // N.B. that this is called only one per suite, not once per test.
   suiteSetup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     this.windowSize = await this.browser.getWindowSize();
     await this.browser.setWindowSize(800, 600);
     await this.browser.pause(PAUSE_TIME);
@@ -41,7 +41,7 @@ suite('Scrolling into view', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    await testSetup(testFileLocations.BASE);
+    await testSetup(testFileLocations.BASE, this.timeout());
   });
 
   test('Insert scrolls new block into view', async function () {

test/webdriverio/test/stack_navigation.ts

@@ -18,7 +18,7 @@ import {
 suite('Stack navigation', function () {
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.COMMENTS);
+    this.browser = await testSetup(testFileLocations.COMMENTS, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/styling_test.ts

@@ -23,7 +23,7 @@ suite('Styling test', function () {
 
   // Clear the workspace and load start blocks.
   setup(async function () {
-    this.browser = await testSetup(testFileLocations.BASE);
+    this.browser = await testSetup(testFileLocations.BASE, this.timeout());
     await this.browser.pause(PAUSE_TIME);
   });
 

test/webdriverio/test/test_setup.ts

@@ -49,10 +49,14 @@ export const PAUSE_TIME = 0;
  * done once, to avoid constantly popping browser windows open and
  * closed.
  *
+ * @param wdioWaitTimeoutMs The timeout for WebdriverIO waitFor*() commands, in
+ *     milliseconds.
  * @returns A Promise that resolves to a WebdriverIO browser that
  *     tests can manipulate.
  */
-export async function driverSetup(): Promise<webdriverio.Browser> {
+export async function driverSetup(
+  wdioWaitTimeoutMs: number,
+): Promise<webdriverio.Browser> {
   const options = {
     capabilities: {
       'browserName': 'chrome',
@@ -66,6 +70,7 @@ export async function driverSetup(): Promise<webdriverio.Browser> {
       // eslint-disable-next-line @typescript-eslint/naming-convention
       'wdio:enforceWebDriverClassic': true,
     },
+    waitforTimeout: wdioWaitTimeoutMs,
     logLevel: 'warn' as const,
   };
 
@@ -104,20 +109,24 @@ export async function driverTeardown() {
  *
  * @param playgroundUrl The URL to open for the test, which should be
  *     a Blockly playground with a workspace.
+ * @param wdioWaitTimeoutMs The timeout for WebdriverIO waitFor*() commands, in
+ *     milliseconds. This should generally be synchronized with the default
+ *     Mocah test timeout.
  * @returns A Promise that resolves to a WebdriverIO browser that
  *     tests can manipulate.
  */
 export async function testSetup(
   playgroundUrl: string,
+  wdioWaitTimeoutMs: number,
 ): Promise<webdriverio.Browser> {
   if (!driver) {
-    driver = await driverSetup();
+    driver = await driverSetup(wdioWaitTimeoutMs);
   }
   await driver.url(playgroundUrl);
   // Wait for the workspace to exist and be rendered.
   await driver
     .$('.blocklySvg .blocklyWorkspace > .blocklyBlockCanvas')
-    .waitForExist({timeout: 2000});
+    .waitForExist();
   return driver;
 }
 
@@ -193,6 +202,7 @@ export async function focusWorkspace(browser: WebdriverIO.Browser) {
     '#blocklyDiv > div > svg.blocklySvg > g',
   );
   await workspaceElement.click({x: 100});
+  await browser.pause(PAUSE_TIME);
 }
 
 /**
@@ -205,7 +215,7 @@ export async function moveToToolboxCategory(
   browser: WebdriverIO.Browser,
   category: string,
 ) {
-  await browser.keys('t');
+  await sendKeyAndWait(browser, 't');
   const categoryIndex = await browser.execute((category) => {
     const all = Array.from(
       document.querySelectorAll('.blocklyToolboxCategoryLabel'),
@@ -275,12 +285,13 @@ export async function focusOnBlock(
   browser: WebdriverIO.Browser,
   blockId: string,
 ) {
-  return await browser.execute((blockId) => {
+  await browser.execute((blockId) => {
     const workspaceSvg = Blockly.getMainWorkspace() as Blockly.WorkspaceSvg;
     const block = workspaceSvg.getBlockById(blockId);
     if (!block) throw new Error(`No block found with ID: ${blockId}.`);
     Blockly.getFocusManager().focusNode(block);
   }, blockId);
+  await browser.pause(PAUSE_TIME);
 }
 
 /**
@@ -295,14 +306,15 @@ export async function focusOnWorkspaceComment(
   browser: WebdriverIO.Browser,
   commentId: string,
 ) {
-  return await browser.execute((commentId) => {
+  await browser.execute((commentId) => {
     const workspaceSvg = Blockly.getMainWorkspace() as Blockly.WorkspaceSvg;
     const comment = workspaceSvg.getCommentById(commentId);
     if (!comment) {
       throw new Error(`No workspace comment found with ID: ${commentId}.`);
     }
     Blockly.getFocusManager().focusNode(comment);
   }, commentId);
+  await browser.pause(PAUSE_TIME);
 }
 
 /**
@@ -320,7 +332,7 @@ export async function focusOnBlockField(
   blockId: string,
   fieldName: string,
 ) {
-  return await browser.execute(
+  await browser.execute(
     (blockId, fieldName) => {
       const workspaceSvg = Blockly.getMainWorkspace() as Blockly.WorkspaceSvg;
       const block = workspaceSvg.getBlockById(blockId);
@@ -334,6 +346,7 @@ export async function focusOnBlockField(
     blockId,
     fieldName,
   );
+  await browser.pause(PAUSE_TIME);
 }
 
 /**
@@ -466,6 +479,7 @@ export async function tabNavigateToWorkspace(
   // there's no straightforward way to do that; see
   // https://stackoverflow.com/q/51518855/4969945
   await browser.execute(() => document.getElementById('focusableDiv')?.focus());
+  await browser.pause(PAUSE_TIME);
   // Navigate to workspace.
   if (hasToolbox) await tabNavigateForward(browser);
   if (hasFlyout) await tabNavigateForward(browser);
@@ -639,7 +653,7 @@ export async function contextMenuExists(
   const item = await browser.$(
     `//div[contains(@class, "blocklyMenuItem")]//*[text()="${itemText}"]`,
   );
-  return await item.waitForExist({timeout: 200, reverse: reverse});
+  return await item.waitForExist({reverse: reverse});
 }
 
 /**
@@ -711,11 +725,13 @@ export async function clickBlock(
     blockId,
     findableId,
   );
+  await browser.pause(PAUSE_TIME);
 
   // In the test context, get the WebdriverIO Element that we've identified.
   const elem = await browser.$(`#${findableId}`);
 
   await elem.click(clickOptions);
+  await browser.pause(PAUSE_TIME);
 
   // In the browser context, remove the ID.
   await browser.execute((elemId) => {
@@ -735,4 +751,5 @@ export async function rightClickOnFlyoutBlockType(
 ) {
   const elem = await browser.$(`.blocklyFlyout .${blockType}`);
   await elem.click({button: 'right'});
+  await browser.pause(PAUSE_TIME);
 }