From 9b18a9b75a6ece875acbe1ba1e37543963a4df99 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Mon, 23 Jun 2025 12:24:10 -0700 Subject: [PATCH 1/9] Work on fixing more browser tests --- tests/browser/test/delete_blocks_test.mjs | 3 +- tests/browser/test/procedure_test.mjs | 14 ++++--- tests/browser/test/test_setup.mjs | 50 +++++++++++++++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.mjs b/tests/browser/test/delete_blocks_test.mjs index a407ad060..133716fa8 100644 --- a/tests/browser/test/delete_blocks_test.mjs +++ b/tests/browser/test/delete_blocks_test.mjs @@ -194,7 +194,8 @@ suite('Delete blocks', function (done) { ); }); - test('Redo block deletion', async function () { + // TODO(#9029) enable this test once deleting a block doesn't lose focus + test.skip('Redo block deletion', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. await clickBlock(this.browser, this.firstBlock.id, {button: 1}); diff --git a/tests/browser/test/procedure_test.mjs b/tests/browser/test/procedure_test.mjs index c01eb4956..c31ef6d73 100644 --- a/tests/browser/test/procedure_test.mjs +++ b/tests/browser/test/procedure_test.mjs @@ -12,6 +12,7 @@ import * as chai from 'chai'; import { connect, getBlockTypeFromCategory, + getDraggableBlockElementByType, getNthBlockOfCategory, getSelectedBlockElement, PAUSE_TIME, @@ -31,9 +32,10 @@ suite('Testing Connecting Blocks', function (done) { this.browser.on('dialog', (dialog) => {}); }); - test('Testing Procedure', async function () { + test.only('Testing Procedure', async function () { + // Drag out first function - let proceduresDefReturn = await getBlockTypeFromCategory( + let proceduresDefReturn = await getDraggableBlockElementByType( this.browser, 'Functions', 'procedures_defreturn', @@ -42,12 +44,12 @@ suite('Testing Connecting Blocks', function (done) { const doSomething = await getSelectedBlockElement(this.browser); // Drag out second function. - proceduresDefReturn = await getBlockTypeFromCategory( + proceduresDefReturn = await getDraggableBlockElementByType( this.browser, 'Functions', 'procedures_defreturn', ); - await proceduresDefReturn.dragAndDrop({x: 300, y: 200}); + await proceduresDefReturn.dragAndDrop({x: 50, y: 20}); const doSomething2 = await getSelectedBlockElement(this.browser); // Drag out numeric @@ -86,7 +88,7 @@ suite('Testing Connecting Blocks', function (done) { 'Text', 'text_print', ); - await printFlyout.dragAndDrop({x: 50, y: 20}); + await printFlyout.dragAndDrop({x: 50, y: 0}); const print = await getSelectedBlockElement(this.browser); // Drag out doSomething2 caller from flyout. @@ -95,7 +97,7 @@ suite('Testing Connecting Blocks', function (done) { 'Functions', 4, ); - await doSomething2Flyout.dragAndDrop({x: 130, y: 20}); + await doSomething2Flyout.dragAndDrop({x: 50, y: 20}); const doSomething2Caller = await getSelectedBlockElement(this.browser); // Connect doSomething2 caller with print. diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index 04a192a46..edfad4d1b 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -286,6 +286,7 @@ export async function getBlockTypeFromCategory( await category.click(); } + await browser.pause(PAUSE_TIME); const id = await browser.execute((blockType) => { return Blockly.getMainWorkspace() .getFlyout() @@ -295,6 +296,55 @@ export async function getBlockTypeFromCategory( return getBlockElementById(browser, id); } +/** + * @param browser The active WebdriverIO Browser object. + * @param categoryName The name of the toolbox category to search. + * Null if the toolbox has no categories (simple). + * @param blockType The type of the block to search for. + * @return A Promise that resolves to a reasonable drag target element of the + * first block with the given type in the given category. + */ +export async function getDraggableBlockElementByType( + browser, + categoryName, + blockType, +) { + if (categoryName) { + const category = await getCategory(browser, categoryName); + await category.click(); + } + + const findableId = 'dragTargetElement'; + // In the browser context, find the element that we want and give it a findable ID. + await browser.execute( + (blockType, newElemId) => { + const block = Blockly.getMainWorkspace() + .getFlyout() + .getWorkspace() + .getBlocksByType(blockType)[0]; + if (!block.isCollapsed()) { + for (const input of block.inputList) { + for (const field of input.fieldRow) { + if (field instanceof Blockly.FieldLabel) { + const svgRoot = field.getSvgRoot(); + if (svgRoot) { + svgRoot.id = newElemId; + return; + } + } + } + } + } + // No label field found. Fall back to the block's SVG root. + block.getSvgRoot().id = newElemId; + }, + blockType, + findableId, + ); + // In the test context, get the Webdriverio Element that we've identified. + return await browser.$(`#${findableId}`); +} + /** * @param browser The active WebdriverIO Browser object. * @param blockType The type of the block to search for in the workspace. From 51bfadba11966fc98d575fce404f4703fe5e1c32 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Fri, 27 Jun 2025 09:41:41 -0700 Subject: [PATCH 2/9] Remove .only --- tests/browser/test/procedure_test.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/browser/test/procedure_test.mjs b/tests/browser/test/procedure_test.mjs index c31ef6d73..ed63beb82 100644 --- a/tests/browser/test/procedure_test.mjs +++ b/tests/browser/test/procedure_test.mjs @@ -32,8 +32,7 @@ suite('Testing Connecting Blocks', function (done) { this.browser.on('dialog', (dialog) => {}); }); - test.only('Testing Procedure', async function () { - + test('Testing Procedure', async function () { // Drag out first function let proceduresDefReturn = await getDraggableBlockElementByType( this.browser, From 77543d3c188793613088b99b6d5aed31827657b1 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Fri, 27 Jun 2025 11:44:09 -0700 Subject: [PATCH 3/9] Fix tests for opening categories --- tests/browser/test/test_setup.mjs | 1 + tests/browser/test/toolbox_drag_test.mjs | 37 +++++++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index edfad4d1b..040ca3df0 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -62,6 +62,7 @@ export async function driverSetup() { // Use Selenium to bring up the page console.log('Starting webdriverio...'); driver = await webdriverio.remote(options); + driver.setWindowSize(800, 600); return driver; } diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index 742872d93..1845336eb 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -11,6 +11,7 @@ import * as chai from 'chai'; import { getCategory, + getDraggableBlockElementByType, PAUSE_TIME, screenDirection, scrollFlyout, @@ -81,6 +82,32 @@ async function elementInBounds(browser, element) { }, element); } +/** + * Get the type of the nth block in the specified category. + * @param browser The active WebdriverIO Browser object. + * @param categoryName The name of the category to inspect. + * @param n The index of the block to get + * @returns A Promise resolving to the type the block in the specified + * category's flyout at index i. + */ +async function getNthBlockType(browser, categoryName, n) { + const category = await getCategory(browser, categoryName); + await category.click(); + await browser.pause(PAUSE_TIME); + + const blockType = await browser.execute((i) => { + return Blockly.getMainWorkspace() + .getFlyout() + .getWorkspace() + .getTopBlocks(false)[i].type; + }, n); + + // Unicode escape to close flyout. + await browser.keys(['\uE00C']); + await browser.pause(PAUSE_TIME); + return blockType; +} + /** * Get how many top-level blocks there are in the specified category. * @param browser The active WebdriverIO Browser object. @@ -145,14 +172,16 @@ async function openCategories(browser, categoryList, directionMultiplier) { await browser.pause(PAUSE_TIME); continue; } - const flyoutBlock = await browser.$( - `.blocklyFlyout .blocklyBlockCanvas > g:nth-child(${3 + i * 2})`, + const blockType = await getNthBlockType(browser, categoryName, i); + const flyoutBlock = await getDraggableBlockElementByType( + browser, + categoryName, + blockType, ); while (!(await elementInBounds(browser, flyoutBlock))) { await scrollFlyout(browser, 0, 50); } - - await flyoutBlock.dragAndDrop({x: directionMultiplier * 50, y: 0}); + await flyoutBlock.click(); await browser.pause(PAUSE_TIME); // Should be one top level block on the workspace. const topBlockCount = await browser.execute(() => { From 3d6ac549a9d82efdd99a152fc4ca2d35eaab6dd7 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Fri, 27 Jun 2025 14:55:31 -0700 Subject: [PATCH 4/9] Fix procedure tests --- tests/browser/test/procedure_test.mjs | 46 +++++++++++------------- tests/browser/test/test_setup.mjs | 36 +++++++++++++++++-- tests/browser/test/toolbox_drag_test.mjs | 38 ++------------------ 3 files changed, 57 insertions(+), 63 deletions(-) diff --git a/tests/browser/test/procedure_test.mjs b/tests/browser/test/procedure_test.mjs index ed63beb82..0b0616acd 100644 --- a/tests/browser/test/procedure_test.mjs +++ b/tests/browser/test/procedure_test.mjs @@ -11,10 +11,8 @@ import * as chai from 'chai'; import { connect, - getBlockTypeFromCategory, - getDraggableBlockElementByType, - getNthBlockOfCategory, - getSelectedBlockElement, + dragBlockTypeFromFlyout, + dragNthBlockFromFlyout, PAUSE_TIME, testFileLocations, testSetup, @@ -34,43 +32,41 @@ suite('Testing Connecting Blocks', function (done) { test('Testing Procedure', async function () { // Drag out first function - let proceduresDefReturn = await getDraggableBlockElementByType( + const doSomething = await dragBlockTypeFromFlyout( this.browser, 'Functions', 'procedures_defreturn', + 50, + 20, ); - await proceduresDefReturn.dragAndDrop({x: 50, y: 20}); - const doSomething = await getSelectedBlockElement(this.browser); - // Drag out second function. - proceduresDefReturn = await getDraggableBlockElementByType( + const doSomething2 = await dragBlockTypeFromFlyout( this.browser, 'Functions', 'procedures_defreturn', + 50, + 20, ); - await proceduresDefReturn.dragAndDrop({x: 50, y: 20}); - const doSomething2 = await getSelectedBlockElement(this.browser); - // Drag out numeric - const mathNumeric = await getBlockTypeFromCategory( + const numeric = await dragBlockTypeFromFlyout( this.browser, 'Math', 'math_number', + 50, + 20, ); - await mathNumeric.dragAndDrop({x: 50, y: 20}); - const numeric = await getSelectedBlockElement(this.browser); // Connect numeric to first procedure await connect(this.browser, numeric, 'OUTPUT', doSomething, 'RETURN'); // Drag out doSomething caller from flyout. - const doSomethingFlyout = await getNthBlockOfCategory( + const doSomethingCaller = await dragNthBlockFromFlyout( this.browser, 'Functions', 3, + 50, + 20, ); - await doSomethingFlyout.dragAndDrop({x: 50, y: 20}); - const doSomethingCaller = await getSelectedBlockElement(this.browser); // Connect the doSomething caller to doSomething2 await connect( @@ -82,22 +78,22 @@ suite('Testing Connecting Blocks', function (done) { ); // Drag out print from flyout. - const printFlyout = await getBlockTypeFromCategory( + const print = await dragBlockTypeFromFlyout( this.browser, 'Text', 'text_print', + 50, + 0, ); - await printFlyout.dragAndDrop({x: 50, y: 0}); - const print = await getSelectedBlockElement(this.browser); // Drag out doSomething2 caller from flyout. - const doSomething2Flyout = await getNthBlockOfCategory( + const doSomething2Caller = await dragNthBlockFromFlyout( this.browser, 'Functions', 4, + 50, + 20, ); - await doSomething2Flyout.dragAndDrop({x: 50, y: 20}); - const doSomething2Caller = await getSelectedBlockElement(this.browser); // Connect doSomething2 caller with print. await connect(this.browser, doSomething2Caller, 'OUTPUT', print, 'TEXT'); @@ -107,7 +103,7 @@ suite('Testing Connecting Blocks', function (done) { runButton.click(); await this.browser.pause(PAUSE_TIME); const alertText = await this.browser.getAlertText(); // get the alert text - chai.assert.equal(alertText, '123'); + chai.assert.equal(alertText, 'abc'); await this.browser.acceptAlert(); }); }); diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index 040ca3df0..2d2525a48 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -216,7 +216,7 @@ export async function clickBlock(browser, blockId, clickOptions) { * @return A Promise that resolves when the actions are completed. */ export async function clickWorkspace(browser) { - const workspace = await browser.$('#blocklyDiv > div > svg.blocklySvg > g'); + const workspace = await browser.$('svg.blocklySvg > g'); await workspace.click(); await browser.pause(PAUSE_TIME); } @@ -499,6 +499,9 @@ export async function switchRTL(browser) { */ export async function dragNthBlockFromFlyout(browser, categoryName, n, x, y) { const flyoutBlock = await getNthBlockOfCategory(browser, categoryName, n); + while (!(await elementInBounds(browser, flyoutBlock))) { + await scrollFlyout(browser, 0, 50); + } await flyoutBlock.dragAndDrop({x: x, y: y}); return await getSelectedBlockElement(browser); } @@ -525,15 +528,44 @@ export async function dragBlockTypeFromFlyout( x, y, ) { - const flyoutBlock = await getBlockTypeFromCategory( + const flyoutBlock = await getDraggableBlockElementByType( browser, categoryName, type, ); + while (!(await elementInBounds(browser, flyoutBlock))) { + await scrollFlyout(browser, 0, 50); + } await flyoutBlock.dragAndDrop({x: x, y: y}); + await browser.pause(PAUSE_TIME); return await getSelectedBlockElement(browser); } +/** + * Check whether an element is fully inside the bounds of the Blockly div. You can use this + * to determine whether a block on the workspace or flyout is inside the Blockly div. + * This does not check whether there are other Blockly elements (such as a toolbox or + * flyout) on top of the element. A partially visible block is considered out of bounds. + * @param browser The active WebdriverIO Browser object. + * @param element The element to look for. + * @returns A Promise resolving to true if the element is in bounds and false otherwise. + */ +async function elementInBounds(browser, element) { + return await browser.execute((elem) => { + const rect = elem.getBoundingClientRect(); + + const blocklyDiv = document.getElementsByClassName('blocklySvg')[0]; + const blocklyRect = blocklyDiv.getBoundingClientRect(); + + const vertInView = + rect.top >= blocklyRect.top && rect.bottom <= blocklyRect.bottom; + const horInView = + rect.left >= blocklyRect.left && rect.right <= blocklyRect.right; + + return vertInView && horInView; + }, element); +} + /** * Drags the specified block type from the mutator flyout of the given block * and returns the root element of the block. diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index 1845336eb..801f5ad78 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -10,11 +10,10 @@ import * as chai from 'chai'; import { + dragBlockTypeFromFlyout, getCategory, - getDraggableBlockElementByType, PAUSE_TIME, screenDirection, - scrollFlyout, testFileLocations, testSetup, } from './test_setup.mjs'; @@ -57,31 +56,6 @@ const testCategories = [ 'Serialization', ]; -/** - * Check whether an element is fully inside the bounds of the Blockly div. You can use this - * to determine whether a block on the workspace or flyout is inside the Blockly div. - * This does not check whether there are other Blockly elements (such as a toolbox or - * flyout) on top of the element. A partially visible block is considered out of bounds. - * @param browser The active WebdriverIO Browser object. - * @param element The element to look for. - * @returns A Promise resolving to true if the element is in bounds and false otherwise. - */ -async function elementInBounds(browser, element) { - return await browser.execute((elem) => { - const rect = elem.getBoundingClientRect(); - - const blocklyDiv = document.getElementById('blocklyDiv'); - const blocklyRect = blocklyDiv.getBoundingClientRect(); - - const vertInView = - rect.top >= blocklyRect.top && rect.bottom <= blocklyRect.bottom; - const horInView = - rect.left >= blocklyRect.left && rect.right <= blocklyRect.right; - - return vertInView && horInView; - }, element); -} - /** * Get the type of the nth block in the specified category. * @param browser The active WebdriverIO Browser object. @@ -173,15 +147,7 @@ async function openCategories(browser, categoryList, directionMultiplier) { continue; } const blockType = await getNthBlockType(browser, categoryName, i); - const flyoutBlock = await getDraggableBlockElementByType( - browser, - categoryName, - blockType, - ); - while (!(await elementInBounds(browser, flyoutBlock))) { - await scrollFlyout(browser, 0, 50); - } - await flyoutBlock.click(); + dragBlockTypeFromFlyout(browser, categoryName, blockType, 50, 20); await browser.pause(PAUSE_TIME); // Should be one top level block on the workspace. const topBlockCount = await browser.execute(() => { From ce3e2514413f58cb468aae1cc4495ff9d3a1832f Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Fri, 27 Jun 2025 15:24:09 -0700 Subject: [PATCH 5/9] Disable test to drag all blocks out and fix comment resize test --- tests/browser/test/procedure_test.mjs | 2 +- tests/browser/test/test_setup.mjs | 1 + tests/browser/test/toolbox_drag_test.mjs | 4 +++- tests/browser/test/workspace_comment_test.mjs | 14 +++++++------- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/browser/test/procedure_test.mjs b/tests/browser/test/procedure_test.mjs index 0b0616acd..d1990fddc 100644 --- a/tests/browser/test/procedure_test.mjs +++ b/tests/browser/test/procedure_test.mjs @@ -103,7 +103,7 @@ suite('Testing Connecting Blocks', function (done) { runButton.click(); await this.browser.pause(PAUSE_TIME); const alertText = await this.browser.getAlertText(); // get the alert text - chai.assert.equal(alertText, 'abc'); + chai.assert.equal(alertText, '123'); await this.browser.acceptAlert(); }); }); diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index 2d2525a48..edbafae42 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -63,6 +63,7 @@ export async function driverSetup() { console.log('Starting webdriverio...'); driver = await webdriverio.remote(options); driver.setWindowSize(800, 600); + driver.setViewport({width: 800, height: 600}); return driver; } diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index 801f5ad78..22d1bb165 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -173,7 +173,9 @@ async function openCategories(browser, categoryList, directionMultiplier) { chai.assert.equal(failureCount, 0); } -suite('Open toolbox categories', function () { +// These take too long to run and are very flakey. Need to find a better way to +// test whatever this is trying to test. +suite.skip('Open toolbox categories', function () { this.timeout(0); test('opening every toolbox category in the category toolbox in LTR', async function () { diff --git a/tests/browser/test/workspace_comment_test.mjs b/tests/browser/test/workspace_comment_test.mjs index 516523276..db42f3099 100644 --- a/tests/browser/test/workspace_comment_test.mjs +++ b/tests/browser/test/workspace_comment_test.mjs @@ -206,13 +206,13 @@ suite('Workspace comments', function () { '.blocklyComment .blocklyResizeHandle', ); await resizeHandle.dragAndDrop(delta); - - chai.assert.deepEqual( - await getCommentSize(this.browser, commentId), - { - width: origSize.width + delta.x, - height: origSize.height + delta.y, - }, + const newSize = await getCommentSize(this.browser, commentId); + chai.assert.isTrue( + Math.abs(newSize.width - (origSize.width + delta.x)) < 1, + 'Expected the comment model size to match the resized size', + ); + chai.assert.isTrue( + Math.abs(newSize.height - (origSize.height + delta.y)) < 1, 'Expected the comment model size to match the resized size', ); }); From b890e32bf98bc00528ad325bdf90272e80d729ab Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Mon, 7 Jul 2025 11:48:55 -0700 Subject: [PATCH 6/9] Re-enable undo/redo tests now that focus is working --- tests/browser/test/delete_blocks_test.mjs | 8 ++++---- tests/browser/test/extensive_test.mjs | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.mjs b/tests/browser/test/delete_blocks_test.mjs index 133716fa8..5c2499c41 100644 --- a/tests/browser/test/delete_blocks_test.mjs +++ b/tests/browser/test/delete_blocks_test.mjs @@ -8,6 +8,7 @@ import * as chai from 'chai'; import {Key} from 'webdriverio'; import { clickBlock, + clickWorkspace, contextMenuSelect, getAllBlocks, getBlockElementById, @@ -176,8 +177,7 @@ suite('Delete blocks', function (done) { ); }); - // TODO(#9029) enable this test once deleting a block doesn't lose focus - test.skip('Undo block deletion', async function () { + test('Undo block deletion', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. await clickBlock(this.browser, this.firstBlock.id, {button: 1}); @@ -194,8 +194,7 @@ suite('Delete blocks', function (done) { ); }); - // TODO(#9029) enable this test once deleting a block doesn't lose focus - test.skip('Redo block deletion', async function () { + test('Redo block deletion', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. await clickBlock(this.browser, this.firstBlock.id, {button: 1}); @@ -205,6 +204,7 @@ suite('Delete blocks', function (done) { await this.browser.keys([Key.Ctrl, 'z']); await this.browser.pause(PAUSE_TIME); // Redo + await clickWorkspace(this.browser); await this.browser.keys([Key.Ctrl, Key.Shift, 'z']); await this.browser.pause(PAUSE_TIME); const after = (await getAllBlocks(this.browser)).length; diff --git a/tests/browser/test/extensive_test.mjs b/tests/browser/test/extensive_test.mjs index bef8bc934..48c066c39 100644 --- a/tests/browser/test/extensive_test.mjs +++ b/tests/browser/test/extensive_test.mjs @@ -40,8 +40,7 @@ suite('This tests loading Large Configuration and Deletion', function (done) { chai.assert.equal(allBlocks.length, 10); }); - // TODO(#8793) Re-enable test after deleting a block updates focus correctly. - test.skip('undoing delete block results in the correct number of blocks', async function () { + test('undoing delete block results in the correct number of blocks', async function () { await this.browser.keys([Key.Ctrl, 'z']); await this.browser.pause(PAUSE_TIME); const allBlocks = await getAllBlocks(this.browser); From 274891d34e9b4bea8afa08f7d89231fce0dc572f Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Tue, 8 Jul 2025 14:27:50 -0700 Subject: [PATCH 7/9] Responses to comments - Switch to using scrollBoundsIntoView instead of scrolling the flyout - Use webdriverio Key.Escape instead of the string code for it --- tests/browser/test/test_setup.mjs | 206 ++++++----------------- tests/browser/test/toolbox_drag_test.mjs | 7 +- 2 files changed, 58 insertions(+), 155 deletions(-) diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index edbafae42..d91a10924 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -172,43 +172,52 @@ export async function getBlockElementById(browser, id) { * @return A Promise that resolves when the actions are completed. */ export async function clickBlock(browser, blockId, clickOptions) { - const findableId = 'clickTargetElement'; // In the browser context, find the element that we want and give it a findable ID. - await browser.execute( - (blockId, newElemId) => { - const block = Blockly.getMainWorkspace().getBlockById(blockId); - // Ensure the block we want to click is within the viewport. - Blockly.getMainWorkspace().scrollBoundsIntoView( - block.getBoundingRectangleWithoutChildren(), - 10, - ); + const elem = await getTargetableBlockElement(browser, blockId, false); + await elem.click(clickOptions); +} + +/** + * Find an element on the block that is suitable for a click or drag. + * + * We can't always use the block's SVG root because clicking will always happen + * in the middle of the block's bounds (including children) by default, which + * causes problems if it has holes (e.g. statement inputs). Instead, this tries + * to get the first text field on the block. It falls back on the block's SVG root. + * @param browser The active WebdriverIO Browser object. + * @param blockId The id of the block to click, as an interactable element. + * @param toolbox True if this block is in the toolbox (which must be open already). + * @return A Promise that returns an appropriate element. + */ +async function getTargetableBlockElement(browser, blockId, toolbox) { + const id = await browser.execute( + (blockId, toolbox, newElemId) => { + const ws = toolbox + ? Blockly.getMainWorkspace().getFlyout().getWorkspace() + : Blockly.getMainWorkspace(); + const block = ws.getBlockById(blockId); + // Ensure the block we want to click/drag is within the viewport. + ws.scrollBoundsIntoView(block.getBoundingRectangleWithoutChildren(), 10); if (!block.isCollapsed()) { for (const input of block.inputList) { for (const field of input.fieldRow) { if (field instanceof Blockly.FieldLabel) { - field.getSvgRoot().id = newElemId; - return; + // Expose the id of the element we want to target + field.getSvgRoot().setAttribute('data-id', field.id_); + return field.getSvgRoot().id; } } } } - // No label field found. Fall back to the block's SVG root. - block.getSvgRoot().id = newElemId; + // No label field found. Fall back to the block's SVG root, which should + // already use the block id. + return block.id; }, blockId, - findableId, + toolbox, ); - // In the test context, get the Webdriverio Element that we've identified. - const elem = await browser.$(`#${findableId}`); - - await elem.click(clickOptions); - - // In the browser context, remove the ID. - await browser.execute((elemId) => { - const clickElem = document.getElementById(elemId); - clickElem.removeAttribute('id'); - }, findableId); + return await getBlockElementById(browser, id); } /** @@ -255,27 +264,14 @@ export async function getCategory(browser, categoryName) { } /** - * @param browser The active WebdriverIO Browser object. - * @param categoryName The name of the toolbox category to search. - * @param n Which block to select, 0-indexed from the top of the category. - * @return A Promise that resolves to the root element of the nth - * block in the given category. - */ -export async function getNthBlockOfCategory(browser, categoryName, n) { - const category = await getCategory(browser, categoryName); - await category.click(); - const block = ( - await browser.$$(`.blocklyFlyout .blocklyBlockCanvas > .blocklyDraggable`) - )[n]; - return block; -} - -/** + * Opens the specified category, finds the first block of the given type, + * scrolls it into view, and returns a draggable element on that block. + * * @param browser The active WebdriverIO Browser object. * @param categoryName The name of the toolbox category to search. * Null if the toolbox has no categories (simple). * @param blockType The type of the block to search for. - * @return A Promise that resolves to the root element of the first + * @return A Promise that resolves to a draggable element of the first * block with the given type in the given category. */ export async function getBlockTypeFromCategory( @@ -290,61 +286,12 @@ export async function getBlockTypeFromCategory( await browser.pause(PAUSE_TIME); const id = await browser.execute((blockType) => { - return Blockly.getMainWorkspace() - .getFlyout() - .getWorkspace() - .getBlocksByType(blockType)[0].id; + const ws = Blockly.getMainWorkspace().getFlyout().getWorkspace(); + const block = ws.getBlocksByType(blockType)[0]; + ws.scrollBoundsIntoView(block.getBoundingRectangleWithoutChildren()); + return block.id; }, blockType); - return getBlockElementById(browser, id); -} - -/** - * @param browser The active WebdriverIO Browser object. - * @param categoryName The name of the toolbox category to search. - * Null if the toolbox has no categories (simple). - * @param blockType The type of the block to search for. - * @return A Promise that resolves to a reasonable drag target element of the - * first block with the given type in the given category. - */ -export async function getDraggableBlockElementByType( - browser, - categoryName, - blockType, -) { - if (categoryName) { - const category = await getCategory(browser, categoryName); - await category.click(); - } - - const findableId = 'dragTargetElement'; - // In the browser context, find the element that we want and give it a findable ID. - await browser.execute( - (blockType, newElemId) => { - const block = Blockly.getMainWorkspace() - .getFlyout() - .getWorkspace() - .getBlocksByType(blockType)[0]; - if (!block.isCollapsed()) { - for (const input of block.inputList) { - for (const field of input.fieldRow) { - if (field instanceof Blockly.FieldLabel) { - const svgRoot = field.getSvgRoot(); - if (svgRoot) { - svgRoot.id = newElemId; - return; - } - } - } - } - } - // No label field found. Fall back to the block's SVG root. - block.getSvgRoot().id = newElemId; - }, - blockType, - findableId, - ); - // In the test context, get the Webdriverio Element that we've identified. - return await browser.$(`#${findableId}`); + return getTargetableBlockElement(browser, id, true); } /** @@ -499,10 +446,16 @@ export async function switchRTL(browser) { * created block. */ export async function dragNthBlockFromFlyout(browser, categoryName, n, x, y) { - const flyoutBlock = await getNthBlockOfCategory(browser, categoryName, n); - while (!(await elementInBounds(browser, flyoutBlock))) { - await scrollFlyout(browser, 0, 50); - } + const category = await getCategory(browser, categoryName); + await category.click(); + + await browser.pause(PAUSE_TIME); + const id = await browser.execute((n) => { + const ws = Blockly.getMainWorkspace().getFlyout().getWorkspace(); + const block = ws.getTopBlocks(true)[n]; + return block.id; + }, n); + const flyoutBlock = await getTargetableBlockElement(browser, id, true); await flyoutBlock.dragAndDrop({x: x, y: y}); return await getSelectedBlockElement(browser); } @@ -529,44 +482,16 @@ export async function dragBlockTypeFromFlyout( x, y, ) { - const flyoutBlock = await getDraggableBlockElementByType( + const flyoutBlock = await getBlockTypeFromCategory( browser, categoryName, type, ); - while (!(await elementInBounds(browser, flyoutBlock))) { - await scrollFlyout(browser, 0, 50); - } await flyoutBlock.dragAndDrop({x: x, y: y}); await browser.pause(PAUSE_TIME); return await getSelectedBlockElement(browser); } -/** - * Check whether an element is fully inside the bounds of the Blockly div. You can use this - * to determine whether a block on the workspace or flyout is inside the Blockly div. - * This does not check whether there are other Blockly elements (such as a toolbox or - * flyout) on top of the element. A partially visible block is considered out of bounds. - * @param browser The active WebdriverIO Browser object. - * @param element The element to look for. - * @returns A Promise resolving to true if the element is in bounds and false otherwise. - */ -async function elementInBounds(browser, element) { - return await browser.execute((elem) => { - const rect = elem.getBoundingClientRect(); - - const blocklyDiv = document.getElementsByClassName('blocklySvg')[0]; - const blocklyRect = blocklyDiv.getBoundingClientRect(); - - const vertInView = - rect.top >= blocklyRect.top && rect.bottom <= blocklyRect.bottom; - const horInView = - rect.left >= blocklyRect.left && rect.right <= blocklyRect.right; - - return vertInView && horInView; - }, element); -} - /** * Drags the specified block type from the mutator flyout of the given block * and returns the root element of the block. @@ -667,27 +592,4 @@ export async function getAllBlocks(browser) { id: block.id, })); }); -} - -/** - * Find the flyout's scrollbar and scroll by the specified amount. - * This makes several assumptions: - * - A flyout with a valid scrollbar exists, is open, and is in view. - * - The workspace has a trash can, which means it has a second (hidden) flyout. - * @param browser The active WebdriverIO Browser object. - * @param xDelta How far to drag the flyout in the x direction. Positive is right. - * @param yDelta How far to drag the flyout in the y direction. Positive is down. - * @return A Promise that resolves when the actions are completed. - */ -export async function scrollFlyout(browser, xDelta, yDelta) { - // There are two flyouts on the playground workspace: one for the trash can - // and one for the toolbox. We want the second one. - // This assumes there is only one scrollbar handle in the flyout, but it could - // be either horizontal or vertical. - await browser.pause(PAUSE_TIME); - const scrollbarHandle = await browser - .$$(`.blocklyFlyoutScrollbar`)[1] - .$(`rect.blocklyScrollbarHandle`); - await scrollbarHandle.dragAndDrop({x: xDelta, y: yDelta}); - await browser.pause(PAUSE_TIME); -} +} \ No newline at end of file diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index 22d1bb165..98d865c1e 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -17,6 +17,7 @@ import { testFileLocations, testSetup, } from './test_setup.mjs'; +import {Key} from 'webdriverio'; // Categories in the basic toolbox. const basicCategories = [ @@ -77,7 +78,7 @@ async function getNthBlockType(browser, categoryName, n) { }, n); // Unicode escape to close flyout. - await browser.keys(['\uE00C']); + await browser.keys([Key.Escape]); await browser.pause(PAUSE_TIME); return blockType; } @@ -102,7 +103,7 @@ async function getBlockCount(browser, categoryName) { }); // Unicode escape to close flyout. - await browser.keys(['\uE00C']); + await browser.keys([Key.Escape]); await browser.pause(PAUSE_TIME); return blockCount; } @@ -142,7 +143,7 @@ async function openCategories(browser, categoryList, directionMultiplier) { await category.click(); if (await isBlockDisabled(browser, i)) { // Unicode escape to close flyout. - await browser.keys(['\uE00C']); + await browser.keys([Key.Escape]); await browser.pause(PAUSE_TIME); continue; } From 1e40641f452cefb1334b95dc0c21e93717170fd9 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Tue, 8 Jul 2025 14:35:28 -0700 Subject: [PATCH 8/9] Fix formatting --- tests/browser/test/test_setup.mjs | 2 +- tests/browser/test/toolbox_drag_test.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/browser/test/test_setup.mjs b/tests/browser/test/test_setup.mjs index d91a10924..6cf4986fc 100644 --- a/tests/browser/test/test_setup.mjs +++ b/tests/browser/test/test_setup.mjs @@ -592,4 +592,4 @@ export async function getAllBlocks(browser) { id: block.id, })); }); -} \ No newline at end of file +} diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index 98d865c1e..be64c6920 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -9,6 +9,7 @@ */ import * as chai from 'chai'; +import {Key} from 'webdriverio'; import { dragBlockTypeFromFlyout, getCategory, @@ -17,7 +18,6 @@ import { testFileLocations, testSetup, } from './test_setup.mjs'; -import {Key} from 'webdriverio'; // Categories in the basic toolbox. const basicCategories = [ From 2fba036a8dd6ae9eec629b0b9f2b6e2437cf2087 Mon Sep 17 00:00:00 2001 From: Erik Pasternak Date: Tue, 8 Jul 2025 15:17:33 -0700 Subject: [PATCH 9/9] Add a todo for enabling the toolbox categories tests --- tests/browser/test/toolbox_drag_test.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/browser/test/toolbox_drag_test.mjs b/tests/browser/test/toolbox_drag_test.mjs index be64c6920..32c201406 100644 --- a/tests/browser/test/toolbox_drag_test.mjs +++ b/tests/browser/test/toolbox_drag_test.mjs @@ -174,8 +174,8 @@ async function openCategories(browser, categoryList, directionMultiplier) { chai.assert.equal(failureCount, 0); } -// These take too long to run and are very flakey. Need to find a better way to -// test whatever this is trying to test. +// TODO (#9217) These take too long to run and are very flakey. Need to find a +// better way to test whatever this is trying to test. suite.skip('Open toolbox categories', function () { this.timeout(0);