From 681ad1a79294838e1ad0a25809f32fc3463cecd2 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Fri, 5 Sep 2025 16:07:05 -0700 Subject: [PATCH 1/8] chore: fix a bug in flaky move test --- test/webdriverio/test/move_test.ts | 38 ++++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index aa441910..de009efb 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -45,12 +45,15 @@ suite('Move start tests', function () { // and block connected to selected block's next connection. const info = await getFocusedNeighbourInfo(this.browser); - chai.assert(info.parentId, 'selected block has no parent block'); + chai.assert.exists( + info.parentId, + 'selected block should have parent block', + ); chai.assert( typeof info.parentIndex === 'number', - 'parent connection index not found', + 'parent connection index should exist and be a number', ); - chai.assert(info.nextId, 'selected block has no next block'); + chai.assert.exists(info.nextId, 'selected block should have next block'); // Start move using keyboard shortcut. await sendKeyAndWait(this.browser, 'm'); @@ -59,12 +62,12 @@ suite('Move start tests', function () { // next/previous connections, and same thing connected to value // input. const newInfo = await getFocusedNeighbourInfo(this.browser); - chai.assert( - newInfo.parentId === null, + chai.assert.isNull( + newInfo.parentId, 'moving block should have no parent block', ); - chai.assert( - newInfo.nextId === null, + chai.assert.isNull( + newInfo.nextId, 'moving block should have no next block', ); chai.assert.strictEqual( @@ -106,16 +109,21 @@ suite('Move start tests', function () { // and block connected to selected block's value input. const info = await getFocusedNeighbourInfo(this.browser); - chai.assert(info.parentId, 'selected block has no parent block'); + chai.assert.exists( + info.parentId, + 'selected block should have parent block', + ); chai.assert( typeof info.parentIndex === 'number', - 'parent connection index not found', + 'parent connection index should exist and be a number', + ); + chai.assert.exists( + info.valueId, + 'selected block should have child value block', ); - chai.assert(info.valueId, 'selected block has no child value block'); // Start move using context menu (using keyboard nav). await sendKeyAndWait(this.browser, [Key.Ctrl, Key.Return]); - await sendKeyAndWait(this.browser, 'm'); await keyDown( this.browser, (await contextMenuItems(this.browser)).findIndex(({text}) => @@ -128,12 +136,12 @@ suite('Move start tests', function () { // next/previous connections, and same thing connected to value // input. const newInfo = await getFocusedNeighbourInfo(this.browser); - chai.assert( - newInfo.parentId === null, + chai.assert.isNull( + newInfo.parentId, 'moving block should have no parent block', ); - chai.assert( - newInfo.nextId === null, + chai.assert.isNull( + newInfo.nextId, 'moving block should have no next block', ); chai.assert.strictEqual( From 09cb7f1a0f059db356b27cd2237bac1843f4a5d5 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Fri, 5 Sep 2025 16:16:35 -0700 Subject: [PATCH 2/8] chore: add assert to make sure context menu is selected --- test/webdriverio/test/move_test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index de009efb..f8f0e49f 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -124,12 +124,17 @@ suite('Move start tests', function () { // Start move using context menu (using keyboard nav). await sendKeyAndWait(this.browser, [Key.Ctrl, Key.Return]); - await keyDown( - this.browser, - (await contextMenuItems(this.browser)).findIndex(({text}) => - text.includes('Move'), - ), + + // Find how many times to press the down arrow + const index = (await contextMenuItems(this.browser)).findIndex(({text}) => + text.includes('Move'), + ); + chai.assert.isAbove( + index, + -1, + 'expected Move to appear in context menu items', ); + await keyDown(this.browser, index); await sendKeyAndWait(this.browser, Key.Return); // Check that the moving block has nothing connected it its From 6a474f740a15e360eb53a0d5a2309a58156d71d3 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Fri, 5 Sep 2025 16:38:24 -0700 Subject: [PATCH 3/8] chore: try using keyboard to start move --- test/webdriverio/test/move_test.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index f8f0e49f..170b1280 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -98,8 +98,6 @@ suite('Move start tests', function () { // When a move of a value block begins, it is expected that block // and all blocks connected to its inputs will be moved - i.e., that // a stack heal (really: unary operator chain heal) will NOT occur. - // - // Also tests initiating a move via the context menu. test('Start moving value blocks', async function () { for (let i = 1; i < 7; i++) { // Navigate to statement_. @@ -122,20 +120,8 @@ suite('Move start tests', function () { 'selected block should have child value block', ); - // Start move using context menu (using keyboard nav). - await sendKeyAndWait(this.browser, [Key.Ctrl, Key.Return]); - - // Find how many times to press the down arrow - const index = (await contextMenuItems(this.browser)).findIndex(({text}) => - text.includes('Move'), - ); - chai.assert.isAbove( - index, - -1, - 'expected Move to appear in context menu items', - ); - await keyDown(this.browser, index); - await sendKeyAndWait(this.browser, Key.Return); + // Start move + await sendKeyAndWait(this.browser, 'm'); // Check that the moving block has nothing connected it its // next/previous connections, and same thing connected to value From c906f12d7ba99affd029129615eb007d03c75b3c Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Fri, 5 Sep 2025 17:05:05 -0700 Subject: [PATCH 4/8] chore: lint --- test/webdriverio/test/move_test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index 170b1280..b4d66ec8 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -15,7 +15,6 @@ import { testSetup, sendKeyAndWait, keyDown, - contextMenuItems, } from './test_setup.js'; suite('Move start tests', function () { From 56df25368ce1032e7e009e776bcc71cee79a04f2 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Mon, 8 Sep 2025 09:08:01 -0700 Subject: [PATCH 5/8] Revert "chore: lint" This reverts commit c906f12d7ba99affd029129615eb007d03c75b3c. --- test/webdriverio/test/move_test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index b4d66ec8..170b1280 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -15,6 +15,7 @@ import { testSetup, sendKeyAndWait, keyDown, + contextMenuItems, } from './test_setup.js'; suite('Move start tests', function () { From 95cdb69adb7e9b80fd7196c7d6d2cad41c29c055 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Mon, 8 Sep 2025 09:09:22 -0700 Subject: [PATCH 6/8] Revert "chore: try using keyboard to start move" This reverts commit 6a474f740a15e360eb53a0d5a2309a58156d71d3. --- test/webdriverio/test/move_test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index 170b1280..f8f0e49f 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -98,6 +98,8 @@ suite('Move start tests', function () { // When a move of a value block begins, it is expected that block // and all blocks connected to its inputs will be moved - i.e., that // a stack heal (really: unary operator chain heal) will NOT occur. + // + // Also tests initiating a move via the context menu. test('Start moving value blocks', async function () { for (let i = 1; i < 7; i++) { // Navigate to statement_. @@ -120,8 +122,20 @@ suite('Move start tests', function () { 'selected block should have child value block', ); - // Start move - await sendKeyAndWait(this.browser, 'm'); + // Start move using context menu (using keyboard nav). + await sendKeyAndWait(this.browser, [Key.Ctrl, Key.Return]); + + // Find how many times to press the down arrow + const index = (await contextMenuItems(this.browser)).findIndex(({text}) => + text.includes('Move'), + ); + chai.assert.isAbove( + index, + -1, + 'expected Move to appear in context menu items', + ); + await keyDown(this.browser, index); + await sendKeyAndWait(this.browser, Key.Return); // Check that the moving block has nothing connected it its // next/previous connections, and same thing connected to value From ab920ddbb9f005a0f77468cd5fd3d4683bca5493 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Mon, 8 Sep 2025 09:36:09 -0700 Subject: [PATCH 7/8] fix: add class to move indicator bubble --- src/move_indicator.ts | 1 + test/webdriverio/test/move_test.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/move_indicator.ts b/src/move_indicator.ts index e6f8e92b..caa5b09e 100644 --- a/src/move_indicator.ts +++ b/src/move_indicator.ts @@ -38,6 +38,7 @@ export class MoveIndicatorBubble {}, workspace.getBubbleCanvas(), ); + this.svgRoot.classList.add('blocklyMoveIndicatorBubble'); const rtl = workspace.RTL; Blockly.utils.dom.createSvgElement( Blockly.utils.Svg.CIRCLE, diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index f8f0e49f..8012ad5a 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -137,6 +137,10 @@ suite('Move start tests', function () { await keyDown(this.browser, index); await sendKeyAndWait(this.browser, Key.Return); + // Wait for the move icon to appear so we know we're in move mode. + const bubble = this.browser.$('.blocklyMoveIndicatorBubble'); + await bubble.waitForExist(); + // Check that the moving block has nothing connected it its // next/previous connections, and same thing connected to value // input. From 190c3d979968a50ee5a91b62387a74d6bc8522e9 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 9 Sep 2025 08:29:34 -0700 Subject: [PATCH 8/8] chore: minor refactor --- test/webdriverio/test/move_test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index 8012ad5a..34c38313 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -138,8 +138,7 @@ suite('Move start tests', function () { await sendKeyAndWait(this.browser, Key.Return); // Wait for the move icon to appear so we know we're in move mode. - const bubble = this.browser.$('.blocklyMoveIndicatorBubble'); - await bubble.waitForExist(); + await this.browser.$('.blocklyMoveIndicatorBubble').waitForExist(); // Check that the moving block has nothing connected it its // next/previous connections, and same thing connected to value