Skip to content

Commit 00d870e

Browse files
Merge pull request #7485 from rachel-fenichel/clickableElement
fix(tests): contextMenuSelect sometimes clicks the wrong block
2 parents fc76981 + 67f42f0 commit 00d870e

3 files changed

Lines changed: 57 additions & 34 deletions

File tree

tests/browser/test/basic_playground_test.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,22 @@ suite('Right Clicking on Blocks', function () {
6969
suiteSetup(async function () {
7070
this.browser = await testSetup(testFileLocations.PLAYGROUND);
7171
this.block = await dragNthBlockFromFlyout(this.browser, 'Loops', 0, 20, 20);
72-
this.blockId = this.block.id;
7372
});
7473

7574
test('clicking the collapse option collapses the block', async function () {
7675
await contextMenuSelect(this.browser, this.block, 'Collapse Block');
77-
chai.assert.isTrue(await getIsCollapsed(this.browser, this.blockId));
76+
chai.assert.isTrue(await getIsCollapsed(this.browser, this.block.id));
7877
});
7978

8079
// Assumes that
8180
test('clicking the expand option expands the block', async function () {
8281
await contextMenuSelect(this.browser, this.block, 'Expand Block');
83-
chai.assert.isFalse(await getIsCollapsed(this.browser, this.blockId));
82+
chai.assert.isFalse(await getIsCollapsed(this.browser, this.block.id));
8483
});
8584

8685
test('clicking the disable option disables the block', async function () {
8786
await contextMenuSelect(this.browser, this.block, 'Disable Block');
88-
chai.assert.isTrue(await getIsDisabled(this.browser, this.blockId));
87+
chai.assert.isTrue(await getIsDisabled(this.browser, this.block.id));
8988
});
9089

9190
test('clicking the enable option enables the block', async function () {

tests/browser/test/delete_blocks_test.js

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
testFileLocations,
1111
getAllBlocks,
1212
getBlockElementById,
13+
clickBlock,
1314
contextMenuSelect,
1415
PAUSE_TIME,
1516
} = require('./test_setup');
@@ -130,15 +131,13 @@ suite('Delete blocks', function (done) {
130131
(await getBlockElementById(this.browser, firstBlockId)).waitForExist({
131132
timeout: 2000,
132133
});
134+
this.firstBlock = await getBlockElementById(this.browser, firstBlockId);
133135
});
134136

135137
test('Delete block using backspace key', async function () {
136138
const before = (await getAllBlocks(this.browser)).length;
137139
// Get first print block, click to select it, and delete it using backspace key.
138-
const block = (await getBlockElementById(this.browser, firstBlockId)).$(
139-
'.blocklyPath',
140-
);
141-
await block.click();
140+
await clickBlock(this.browser, this.firstBlock, {button: 1});
142141
await this.browser.keys([Key.Backspace]);
143142
const after = (await getAllBlocks(this.browser)).length;
144143
chai.assert.equal(
@@ -151,10 +150,7 @@ suite('Delete blocks', function (done) {
151150
test('Delete block using delete key', async function () {
152151
const before = (await getAllBlocks(this.browser)).length;
153152
// Get first print block, click to select it, and delete it using delete key.
154-
const block = (await getBlockElementById(this.browser, firstBlockId)).$(
155-
'.blocklyPath',
156-
);
157-
await block.click();
153+
await clickBlock(this.browser, this.firstBlock, {button: 1});
158154
await this.browser.keys([Key.Delete]);
159155
const after = (await getAllBlocks(this.browser)).length;
160156
chai.assert.equal(
@@ -167,8 +163,7 @@ suite('Delete blocks', function (done) {
167163
test('Delete block using context menu', async function () {
168164
const before = (await getAllBlocks(this.browser)).length;
169165
// Get first print block, click to select it, and delete it using context menu.
170-
const block = await getBlockElementById(this.browser, firstBlockId);
171-
await contextMenuSelect(this.browser, block, 'Delete 2 Blocks');
166+
await contextMenuSelect(this.browser, this.firstBlock, 'Delete 2 Blocks');
172167
const after = (await getAllBlocks(this.browser)).length;
173168
chai.assert.equal(
174169
before - 2,
@@ -180,10 +175,7 @@ suite('Delete blocks', function (done) {
180175
test('Undo block deletion', async function () {
181176
const before = (await getAllBlocks(this.browser)).length;
182177
// Get first print block, click to select it, and delete it using backspace key.
183-
const block = (await getBlockElementById(this.browser, firstBlockId)).$(
184-
'.blocklyPath',
185-
);
186-
await block.click();
178+
await clickBlock(this.browser, this.firstBlock, {button: 1});
187179
await this.browser.keys([Key.Backspace]);
188180
await this.browser.pause(PAUSE_TIME);
189181
// Undo
@@ -199,10 +191,7 @@ suite('Delete blocks', function (done) {
199191
test('Redo block deletion', async function () {
200192
const before = (await getAllBlocks(this.browser)).length;
201193
// Get first print block, click to select it, and delete it using backspace key.
202-
const block = (await getBlockElementById(this.browser, firstBlockId)).$(
203-
'.blocklyPath',
204-
);
205-
await block.click();
194+
await clickBlock(this.browser, this.firstBlock, {button: 1});
206195
await this.browser.keys([Key.Backspace]);
207196
await this.browser.pause(PAUSE_TIME);
208197
// Undo

tests/browser/test/test_setup.js

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,50 @@ async function getBlockElementById(browser, id) {
155155
return elem;
156156
}
157157

158+
/**
159+
* Find a clickable element on the block and click it.
160+
* We can't always use the block's SVG root because clicking will always happen
161+
* in the middle of the block's bounds (including children) by default, which
162+
* causes problems if it has holes (e.g. statement inputs). Instead, this tries
163+
* to get the first text field on the block. It falls back on the block's SVG root.
164+
* @param browser The active WebdriverIO Browser object.
165+
* @param block The block to click, as an interactable element.
166+
* @param clickOptions The options to pass to webdriverio's element.click function.
167+
* @return A Promise that resolves when the actions are completed.
168+
*/
169+
async function clickBlock(browser, block, clickOptions) {
170+
const findableId = 'clickTargetElement';
171+
// In the browser context, find the element that we want and give it a findable ID.
172+
await browser.execute(
173+
(blockId, newElemId) => {
174+
const block = Blockly.getMainWorkspace().getBlockById(blockId);
175+
for (const input of block.inputList) {
176+
for (const field of input.fieldRow) {
177+
if (field instanceof Blockly.FieldLabel) {
178+
field.getSvgRoot().id = newElemId;
179+
return;
180+
}
181+
}
182+
}
183+
// No label field found. Fall back to the block's SVG root.
184+
block.getSvgRoot().id = findableId;
185+
},
186+
block.id,
187+
findableId,
188+
);
189+
190+
// In the test context, get the Webdriverio Element that we've identified.
191+
const elem = await browser.$(`#${findableId}`);
192+
193+
await elem.click(clickOptions);
194+
195+
// In the browser context, remove the ID.
196+
await browser.execute((elemId) => {
197+
const clickElem = document.getElementById(elemId);
198+
clickElem.removeAttribute('id');
199+
}, findableId);
200+
}
201+
158202
/**
159203
* @param browser The active WebdriverIO Browser object.
160204
* @param categoryName The name of the toolbox category to find.
@@ -428,23 +472,13 @@ async function dragBlockFromMutatorFlyout(browser, mutatorBlock, type, x, y) {
428472
* context menu item.
429473
*
430474
* @param browser The active WebdriverIO Browser object.
431-
* @param block The block to click, as an interactable element. This block must
475+
* @param block The block to click, as an interactable element. This block should
432476
* have text on it, because we use the text element as the click target.
433477
* @param itemText The display text of the context menu item to click.
434478
* @return A Promise that resolves when the actions are completed.
435479
*/
436480
async function contextMenuSelect(browser, block, itemText) {
437-
// Clicking will always happen in the middle of the block's bounds
438-
// (including children) by default, which causes problems if it has holes
439-
// (e.g. statement inputs).
440-
// Instead, we'll click directly on the first bit of text on the block.
441-
const clickEl = block.$('.blocklyText');
442-
443-
// Even though the element should definitely already exist,
444-
// one specific test breaks if you remove this...
445-
await clickEl.waitForExist();
446-
447-
await clickEl.click({button: 2});
481+
await clickBlock(browser, block, {button: 2});
448482

449483
const item = await browser.$(`div=${itemText}`);
450484
await item.waitForExist();
@@ -515,6 +549,7 @@ module.exports = {
515549
getSelectedBlockElement,
516550
getSelectedBlockId,
517551
getBlockElementById,
552+
clickBlock,
518553
getCategory,
519554
getNthBlockOfCategory,
520555
getBlockTypeFromCategory,

0 commit comments

Comments
 (0)