From 6d63f3cc6b6931275afbb70bfb70cb74f6d7df86 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 12 Sep 2023 14:30:11 -0700 Subject: [PATCH 1/7] feat(tests): add helper to get a clickable element on a block --- tests/browser/test/test_setup.js | 39 +++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/tests/browser/test/test_setup.js b/tests/browser/test/test_setup.js index 69e851b6c..9b8919acb 100644 --- a/tests/browser/test/test_setup.js +++ b/tests/browser/test/test_setup.js @@ -155,6 +155,38 @@ async function getBlockElementById(browser, id) { return elem; } +/** + * Get a clickable element on the block. 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 id The ID of the Blockly block to search for. + * @return A Promise that resolves to the text element of the first label + * field on the block, or the block's SVG root if no label field was found. + */ +async function getClickableBlockElementById(browser, id) { + // In the browser context, find the element that we want and give it a findable ID. + await browser.execute((blockId) => { + const block = Blockly.getMainWorkspace().getBlockById(blockId); + for (const input of block.inputList) { + for (const field of input.fieldRow) { + if (field instanceof Blockly.FieldLabel) { + field.getSvgRoot().id = 'clickTargetElement'; + return; + } + } + } + // No label field found. Fall back to the block's SVG root. + block.getSvgRoot().id = 'clickTargetElement'; + }, id); + + // In the test context, get the Webdriverio Element that we've identified. + const elem = await browser.$('#clickTargetElement'); + return elem; +} + /** * @param browser The active WebdriverIO Browser object. * @param categoryName The name of the toolbox category to find. @@ -434,12 +466,7 @@ async function dragBlockFromMutatorFlyout(browser, mutatorBlock, type, x, y) { * @return A Promise that resolves when the actions are completed. */ async function contextMenuSelect(browser, block, itemText) { - // 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, we'll click directly on the first bit of text on the block. - const clickEl = block.$('.blocklyText'); - + const clickEl = await getClickableBlockElementById(browser, block.id); // Even though the element should definitely already exist, // one specific test breaks if you remove this... await clickEl.waitForExist(); From 1f5dc25b3cc1d361584abfd6e8e006e6ddd2a977 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 12 Sep 2023 14:43:31 -0700 Subject: [PATCH 2/7] fix(tests): use new helper function for right-clicks --- tests/browser/test/basic_playground_test.js | 25 ++++++++++----------- tests/browser/test/delete_blocks_test.js | 2 +- tests/browser/test/test_setup.js | 6 ++--- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/browser/test/basic_playground_test.js b/tests/browser/test/basic_playground_test.js index 965de4445..ef8bc4abb 100644 --- a/tests/browser/test/basic_playground_test.js +++ b/tests/browser/test/basic_playground_test.js @@ -69,37 +69,36 @@ suite('Right Clicking on Blocks', function () { suiteSetup(async function () { this.browser = await testSetup(testFileLocations.PLAYGROUND); this.block = await dragNthBlockFromFlyout(this.browser, 'Loops', 0, 20, 20); - this.blockId = this.block.id; }); test('clicking the collapse option collapses the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Collapse Block'); - chai.assert.isTrue(await getIsCollapsed(this.browser, this.blockId)); + await contextMenuSelect(this.browser, this.block.id, 'Collapse Block'); + chai.assert.isTrue(await getIsCollapsed(this.browser, this.block.id)); }); // Assumes that test('clicking the expand option expands the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Expand Block'); - chai.assert.isFalse(await getIsCollapsed(this.browser, this.blockId)); + await contextMenuSelect(this.browser, this.block.id, 'Expand Block'); + chai.assert.isFalse(await getIsCollapsed(this.browser, this.block.id)); }); test('clicking the disable option disables the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Disable Block'); - chai.assert.isTrue(await getIsDisabled(this.browser, this.blockId)); + await contextMenuSelect(this.browser, this.block.id, 'Disable Block'); + chai.assert.isTrue(await getIsDisabled(this.browser, this.block.id)); }); test('clicking the enable option enables the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Enable Block'); + await contextMenuSelect(this.browser, this.block.id, 'Enable Block'); chai.assert.isFalse(await getIsDisabled(this.browser, this.block.id)); }); test('clicking the add comment option adds a comment to the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Add Comment'); + await contextMenuSelect(this.browser, this.block.id, 'Add Comment'); chai.assert.equal(await getCommentText(this.browser, this.block.id), ''); }); test('clicking the remove comment option removes a comment from the block', async function () { - await contextMenuSelect(this.browser, this.block, 'Remove Comment'); + await contextMenuSelect(this.browser, this.block.id, 'Remove Comment'); chai.assert.isNull(await getCommentText(this.browser, this.block.id)); }); }); @@ -139,7 +138,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'OUTPUT', parent, 'IF0'); - await contextMenuSelect(this.browser, parent, 'Disable Block'); + await contextMenuSelect(this.browser, parent.id, 'Disable Block'); chai.assert.isTrue(await getIsDisabled(this.browser, child.id)); }, @@ -165,7 +164,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'PREVIOUS', parent, 'DO0'); - await contextMenuSelect(this.browser, parent, 'Disable Block'); + await contextMenuSelect(this.browser, parent.id, 'Disable Block'); chai.assert.isTrue(await getIsDisabled(this.browser, child.id)); }, @@ -191,7 +190,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'PREVIOUS', parent, 'NEXT'); - await contextMenuSelect(this.browser, parent, 'Disable Block'); + await contextMenuSelect(this.browser, parent.id, 'Disable Block'); chai.assert.isFalse(await getIsDisabled(this.browser, child.id)); }, diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index 261074a91..e493b840b 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -168,7 +168,7 @@ suite('Delete blocks', function (done) { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using context menu. const block = await getBlockElementById(this.browser, firstBlockId); - await contextMenuSelect(this.browser, block, 'Delete 2 Blocks'); + await contextMenuSelect(this.browser, block.id, 'Delete 2 Blocks'); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( before - 2, diff --git a/tests/browser/test/test_setup.js b/tests/browser/test/test_setup.js index 9b8919acb..055ac2249 100644 --- a/tests/browser/test/test_setup.js +++ b/tests/browser/test/test_setup.js @@ -460,13 +460,13 @@ async function dragBlockFromMutatorFlyout(browser, mutatorBlock, type, x, y) { * context menu item. * * @param browser The active WebdriverIO Browser object. - * @param block The block to click, as an interactable element. This block must + * @param blockId The ID of the block to click. This block should * have text on it, because we use the text element as the click target. * @param itemText The display text of the context menu item to click. * @return A Promise that resolves when the actions are completed. */ -async function contextMenuSelect(browser, block, itemText) { - const clickEl = await getClickableBlockElementById(browser, block.id); +async function contextMenuSelect(browser, blockId, itemText) { + const clickEl = await getClickableBlockElementById(browser, blockId); // Even though the element should definitely already exist, // one specific test breaks if you remove this... await clickEl.waitForExist(); From 6a920a87ccf1a3e47ddd069bccf4f11cc86677d5 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 12 Sep 2023 14:45:08 -0700 Subject: [PATCH 3/7] fix(tests): use new helper function for right-clicks --- tests/browser/test/delete_blocks_test.js | 12 +++++++----- tests/browser/test/test_setup.js | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index e493b840b..843003d77 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -10,6 +10,7 @@ const { testFileLocations, getAllBlocks, getBlockElementById, + getClickableBlockElementById, contextMenuSelect, PAUSE_TIME, } = require('./test_setup'); @@ -132,13 +133,14 @@ suite('Delete blocks', function (done) { }); }); - test('Delete block using backspace key', async function () { + test.only('Delete block using backspace key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. - const block = (await getBlockElementById(this.browser, firstBlockId)).$( - '.blocklyPath', - ); - await block.click(); + // const block = (await getBlockElementById(this.browser, firstBlockId)).$( + // '.blocklyPath', + // ); + const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + await clickEl.click(); await this.browser.keys([Key.Backspace]); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( diff --git a/tests/browser/test/test_setup.js b/tests/browser/test/test_setup.js index 055ac2249..b7819888e 100644 --- a/tests/browser/test/test_setup.js +++ b/tests/browser/test/test_setup.js @@ -542,6 +542,7 @@ module.exports = { getSelectedBlockElement, getSelectedBlockId, getBlockElementById, + getClickableBlockElementById, getCategory, getNthBlockOfCategory, getBlockTypeFromCategory, From 8d6eb80290cff62dfc6a67637f45a6c4a307675a Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 12 Sep 2023 14:49:51 -0700 Subject: [PATCH 4/7] chore(tests): use new helper function in delete tests --- tests/browser/test/delete_blocks_test.js | 26 ++++++++---------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index 843003d77..884ed9d8b 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -133,12 +133,9 @@ suite('Delete blocks', function (done) { }); }); - test.only('Delete block using backspace key', async function () { + test('Delete block using backspace key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. - // const block = (await getBlockElementById(this.browser, firstBlockId)).$( - // '.blocklyPath', - // ); const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); await clickEl.click(); await this.browser.keys([Key.Backspace]); @@ -153,10 +150,8 @@ suite('Delete blocks', function (done) { test('Delete block using delete key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using delete key. - const block = (await getBlockElementById(this.browser, firstBlockId)).$( - '.blocklyPath', - ); - await block.click(); + const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + await clickEl.click(); await this.browser.keys([Key.Delete]); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( @@ -169,8 +164,7 @@ suite('Delete blocks', function (done) { test('Delete block using context menu', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using context menu. - const block = await getBlockElementById(this.browser, firstBlockId); - await contextMenuSelect(this.browser, block.id, 'Delete 2 Blocks'); + await contextMenuSelect(this.browser, firstBlockId, 'Delete 2 Blocks'); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( before - 2, @@ -182,10 +176,8 @@ suite('Delete blocks', function (done) { 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. - const block = (await getBlockElementById(this.browser, firstBlockId)).$( - '.blocklyPath', - ); - await block.click(); + const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + await clickEl.click(); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); // Undo @@ -201,10 +193,8 @@ suite('Delete blocks', function (done) { 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. - const block = (await getBlockElementById(this.browser, firstBlockId)).$( - '.blocklyPath', - ); - await block.click(); + const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + await clickEl.click(); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); // Undo From cad3d4b10c9d8936ba63f566448518192410b5b7 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 12 Sep 2023 15:13:43 -0700 Subject: [PATCH 5/7] chore(tests): format --- tests/browser/test/delete_blocks_test.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index 884ed9d8b..6defcb554 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -136,7 +136,10 @@ suite('Delete blocks', function (done) { test('Delete block using backspace key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. - const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + const clickEl = await getClickableBlockElementById( + this.browser, + firstBlockId, + ); await clickEl.click(); await this.browser.keys([Key.Backspace]); const after = (await getAllBlocks(this.browser)).length; @@ -150,7 +153,10 @@ suite('Delete blocks', function (done) { test('Delete block using delete key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using delete key. - const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + const clickEl = await getClickableBlockElementById( + this.browser, + firstBlockId, + ); await clickEl.click(); await this.browser.keys([Key.Delete]); const after = (await getAllBlocks(this.browser)).length; @@ -176,7 +182,10 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + const clickEl = await getClickableBlockElementById( + this.browser, + firstBlockId, + ); await clickEl.click(); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); @@ -193,7 +202,10 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElementById(this.browser, firstBlockId); + const clickEl = await getClickableBlockElementById( + this.browser, + firstBlockId, + ); await clickEl.click(); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); From 5ee26c9051898878393074ba7d4480129772b373 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 13 Sep 2023 11:30:46 -0700 Subject: [PATCH 6/7] feat: use block instead of block ID in getClickableBlockElement --- tests/browser/test/basic_playground_test.js | 18 +++++++++--------- tests/browser/test/delete_blocks_test.js | 21 +++++++++++---------- tests/browser/test/test_setup.js | 14 +++++++------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/tests/browser/test/basic_playground_test.js b/tests/browser/test/basic_playground_test.js index ef8bc4abb..091f2f6b5 100644 --- a/tests/browser/test/basic_playground_test.js +++ b/tests/browser/test/basic_playground_test.js @@ -72,33 +72,33 @@ suite('Right Clicking on Blocks', function () { }); test('clicking the collapse option collapses the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Collapse Block'); + await contextMenuSelect(this.browser, this.block, 'Collapse Block'); chai.assert.isTrue(await getIsCollapsed(this.browser, this.block.id)); }); // Assumes that test('clicking the expand option expands the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Expand Block'); + await contextMenuSelect(this.browser, this.block, 'Expand Block'); chai.assert.isFalse(await getIsCollapsed(this.browser, this.block.id)); }); test('clicking the disable option disables the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Disable Block'); + await contextMenuSelect(this.browser, this.block, 'Disable Block'); chai.assert.isTrue(await getIsDisabled(this.browser, this.block.id)); }); test('clicking the enable option enables the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Enable Block'); + await contextMenuSelect(this.browser, this.block, 'Enable Block'); chai.assert.isFalse(await getIsDisabled(this.browser, this.block.id)); }); test('clicking the add comment option adds a comment to the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Add Comment'); + await contextMenuSelect(this.browser, this.block, 'Add Comment'); chai.assert.equal(await getCommentText(this.browser, this.block.id), ''); }); test('clicking the remove comment option removes a comment from the block', async function () { - await contextMenuSelect(this.browser, this.block.id, 'Remove Comment'); + await contextMenuSelect(this.browser, this.block, 'Remove Comment'); chai.assert.isNull(await getCommentText(this.browser, this.block.id)); }); }); @@ -138,7 +138,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'OUTPUT', parent, 'IF0'); - await contextMenuSelect(this.browser, parent.id, 'Disable Block'); + await contextMenuSelect(this.browser, parent, 'Disable Block'); chai.assert.isTrue(await getIsDisabled(this.browser, child.id)); }, @@ -164,7 +164,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'PREVIOUS', parent, 'DO0'); - await contextMenuSelect(this.browser, parent.id, 'Disable Block'); + await contextMenuSelect(this.browser, parent, 'Disable Block'); chai.assert.isTrue(await getIsDisabled(this.browser, child.id)); }, @@ -190,7 +190,7 @@ suite('Disabling', function () { ); await connect(this.browser, child, 'PREVIOUS', parent, 'NEXT'); - await contextMenuSelect(this.browser, parent.id, 'Disable Block'); + await contextMenuSelect(this.browser, parent, 'Disable Block'); chai.assert.isFalse(await getIsDisabled(this.browser, child.id)); }, diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index 6defcb554..b5b6ae6f4 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -10,7 +10,7 @@ const { testFileLocations, getAllBlocks, getBlockElementById, - getClickableBlockElementById, + getClickableBlockElement, contextMenuSelect, PAUSE_TIME, } = require('./test_setup'); @@ -131,14 +131,15 @@ suite('Delete blocks', function (done) { (await getBlockElementById(this.browser, firstBlockId)).waitForExist({ timeout: 2000, }); + this.firstBlock = await getBlockElementById(this.browser, firstBlockId); }); test('Delete block using backspace key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. - const clickEl = await getClickableBlockElementById( + const clickEl = await getClickableBlockElement( this.browser, - firstBlockId, + this.firstBlock, ); await clickEl.click(); await this.browser.keys([Key.Backspace]); @@ -153,9 +154,9 @@ suite('Delete blocks', function (done) { test('Delete block using delete key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using delete key. - const clickEl = await getClickableBlockElementById( + const clickEl = await getClickableBlockElement( this.browser, - firstBlockId, + this.firstBlock, ); await clickEl.click(); await this.browser.keys([Key.Delete]); @@ -170,7 +171,7 @@ suite('Delete blocks', function (done) { test('Delete block using context menu', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using context menu. - await contextMenuSelect(this.browser, firstBlockId, 'Delete 2 Blocks'); + await contextMenuSelect(this.browser, this.firstBlock, 'Delete 2 Blocks'); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( before - 2, @@ -182,9 +183,9 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElementById( + const clickEl = await getClickableBlockElement( this.browser, - firstBlockId, + this.firstBlock, ); await clickEl.click(); await this.browser.keys([Key.Backspace]); @@ -202,9 +203,9 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElementById( + const clickEl = await getClickableBlockElement( this.browser, - firstBlockId, + this.firstBlock, ); await clickEl.click(); await this.browser.keys([Key.Backspace]); diff --git a/tests/browser/test/test_setup.js b/tests/browser/test/test_setup.js index b7819888e..4e2db8fdb 100644 --- a/tests/browser/test/test_setup.js +++ b/tests/browser/test/test_setup.js @@ -162,11 +162,11 @@ async function getBlockElementById(browser, id) { * (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 id The ID of the Blockly block to search for. + * @param block The block to click, as an interactable element. * @return A Promise that resolves to the text element of the first label * field on the block, or the block's SVG root if no label field was found. */ -async function getClickableBlockElementById(browser, id) { +async function getClickableBlockElement(browser, block) { // In the browser context, find the element that we want and give it a findable ID. await browser.execute((blockId) => { const block = Blockly.getMainWorkspace().getBlockById(blockId); @@ -180,7 +180,7 @@ async function getClickableBlockElementById(browser, id) { } // No label field found. Fall back to the block's SVG root. block.getSvgRoot().id = 'clickTargetElement'; - }, id); + }, block.id); // In the test context, get the Webdriverio Element that we've identified. const elem = await browser.$('#clickTargetElement'); @@ -460,13 +460,13 @@ async function dragBlockFromMutatorFlyout(browser, mutatorBlock, type, x, y) { * context menu item. * * @param browser The active WebdriverIO Browser object. - * @param blockId The ID of the block to click. This block should + * @param block The block to click, as an interactable element. This block should * have text on it, because we use the text element as the click target. * @param itemText The display text of the context menu item to click. * @return A Promise that resolves when the actions are completed. */ -async function contextMenuSelect(browser, blockId, itemText) { - const clickEl = await getClickableBlockElementById(browser, blockId); +async function contextMenuSelect(browser, block, itemText) { + const clickEl = await getClickableBlockElement(browser, block); // Even though the element should definitely already exist, // one specific test breaks if you remove this... await clickEl.waitForExist(); @@ -542,7 +542,7 @@ module.exports = { getSelectedBlockElement, getSelectedBlockId, getBlockElementById, - getClickableBlockElementById, + getClickableBlockElement, getCategory, getNthBlockOfCategory, getBlockTypeFromCategory, From 67f42f017a32043b625b7958acab71a9a4d4deb4 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 13 Sep 2023 12:36:39 -0700 Subject: [PATCH 7/7] chore(tests): rewrite getClickableBlockElement as clickBlock --- tests/browser/test/delete_blocks_test.js | 26 ++-------- tests/browser/test/test_setup.js | 63 +++++++++++++----------- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/tests/browser/test/delete_blocks_test.js b/tests/browser/test/delete_blocks_test.js index b5b6ae6f4..cecee8ba1 100644 --- a/tests/browser/test/delete_blocks_test.js +++ b/tests/browser/test/delete_blocks_test.js @@ -10,7 +10,7 @@ const { testFileLocations, getAllBlocks, getBlockElementById, - getClickableBlockElement, + clickBlock, contextMenuSelect, PAUSE_TIME, } = require('./test_setup'); @@ -137,11 +137,7 @@ suite('Delete blocks', function (done) { test('Delete block using backspace key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using backspace key. - const clickEl = await getClickableBlockElement( - this.browser, - this.firstBlock, - ); - await clickEl.click(); + await clickBlock(this.browser, this.firstBlock, {button: 1}); await this.browser.keys([Key.Backspace]); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( @@ -154,11 +150,7 @@ suite('Delete blocks', function (done) { test('Delete block using delete key', async function () { const before = (await getAllBlocks(this.browser)).length; // Get first print block, click to select it, and delete it using delete key. - const clickEl = await getClickableBlockElement( - this.browser, - this.firstBlock, - ); - await clickEl.click(); + await clickBlock(this.browser, this.firstBlock, {button: 1}); await this.browser.keys([Key.Delete]); const after = (await getAllBlocks(this.browser)).length; chai.assert.equal( @@ -183,11 +175,7 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElement( - this.browser, - this.firstBlock, - ); - await clickEl.click(); + await clickBlock(this.browser, this.firstBlock, {button: 1}); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); // Undo @@ -203,11 +191,7 @@ suite('Delete blocks', function (done) { 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. - const clickEl = await getClickableBlockElement( - this.browser, - this.firstBlock, - ); - await clickEl.click(); + await clickBlock(this.browser, this.firstBlock, {button: 1}); await this.browser.keys([Key.Backspace]); await this.browser.pause(PAUSE_TIME); // Undo diff --git a/tests/browser/test/test_setup.js b/tests/browser/test/test_setup.js index 4e2db8fdb..ca3f211bb 100644 --- a/tests/browser/test/test_setup.js +++ b/tests/browser/test/test_setup.js @@ -156,35 +156,47 @@ async function getBlockElementById(browser, id) { } /** - * Get a clickable element on the block. 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. + * Find a clickable element on the block and click it. + * 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 block The block to click, as an interactable element. - * @return A Promise that resolves to the text element of the first label - * field on the block, or the block's SVG root if no label field was found. + * @param clickOptions The options to pass to webdriverio's element.click function. + * @return A Promise that resolves when the actions are completed. */ -async function getClickableBlockElement(browser, block) { +async function clickBlock(browser, block, clickOptions) { + const findableId = 'clickTargetElement'; // In the browser context, find the element that we want and give it a findable ID. - await browser.execute((blockId) => { - const block = Blockly.getMainWorkspace().getBlockById(blockId); - for (const input of block.inputList) { - for (const field of input.fieldRow) { - if (field instanceof Blockly.FieldLabel) { - field.getSvgRoot().id = 'clickTargetElement'; - return; + await browser.execute( + (blockId, newElemId) => { + const block = Blockly.getMainWorkspace().getBlockById(blockId); + for (const input of block.inputList) { + for (const field of input.fieldRow) { + if (field instanceof Blockly.FieldLabel) { + field.getSvgRoot().id = newElemId; + return; + } } } - } - // No label field found. Fall back to the block's SVG root. - block.getSvgRoot().id = 'clickTargetElement'; - }, block.id); + // No label field found. Fall back to the block's SVG root. + block.getSvgRoot().id = findableId; + }, + block.id, + findableId, + ); // In the test context, get the Webdriverio Element that we've identified. - const elem = await browser.$('#clickTargetElement'); - return elem; + 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); } /** @@ -466,12 +478,7 @@ async function dragBlockFromMutatorFlyout(browser, mutatorBlock, type, x, y) { * @return A Promise that resolves when the actions are completed. */ async function contextMenuSelect(browser, block, itemText) { - const clickEl = await getClickableBlockElement(browser, block); - // Even though the element should definitely already exist, - // one specific test breaks if you remove this... - await clickEl.waitForExist(); - - await clickEl.click({button: 2}); + await clickBlock(browser, block, {button: 2}); const item = await browser.$(`div=${itemText}`); await item.waitForExist(); @@ -542,7 +549,7 @@ module.exports = { getSelectedBlockElement, getSelectedBlockId, getBlockElementById, - getClickableBlockElement, + clickBlock, getCategory, getNthBlockOfCategory, getBlockTypeFromCategory,