Skip to content

Commit

Permalink
Merge pull request #7485 from rachel-fenichel/clickableElement
Browse files Browse the repository at this point in the history 
fix(tests): contextMenuSelect sometimes clicks the wrong block
  • Loading branch information
@rachel-fenichel
rachel-fenichel authored Sep 13, 2023
2 parents fc76981 + 67f42f0 commit 00d870e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 34 deletions.
7 changes: 3 additions & 4 deletions tests/browser/test/basic_playground_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,22 @@ 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));
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));
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));
chai.assert.isTrue(await getIsDisabled(this.browser, this.block.id));
});

test('clicking the enable option enables the block', async function () {
Expand Down
25 changes: 7 additions & 18 deletions tests/browser/test/delete_blocks_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
testFileLocations,
getAllBlocks,
getBlockElementById,
clickBlock,
contextMenuSelect,
PAUSE_TIME,
} = require('./test_setup');
Expand Down Expand Up @@ -130,15 +131,13 @@ 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 block = (await getBlockElementById(this.browser, firstBlockId)).$(
'.blocklyPath',
);
await block.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(
Expand All @@ -151,10 +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 block = (await getBlockElementById(this.browser, firstBlockId)).$(
'.blocklyPath',
);
await block.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(
Expand All @@ -167,8 +163,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, 'Delete 2 Blocks');
await contextMenuSelect(this.browser, this.firstBlock, 'Delete 2 Blocks');
const after = (await getAllBlocks(this.browser)).length;
chai.assert.equal(
before - 2,
Expand All @@ -180,10 +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 block = (await getBlockElementById(this.browser, firstBlockId)).$(
'.blocklyPath',
);
await block.click();
await clickBlock(this.browser, this.firstBlock, {button: 1});
await this.browser.keys([Key.Backspace]);
await this.browser.pause(PAUSE_TIME);
// Undo
Expand All @@ -199,10 +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 block = (await getBlockElementById(this.browser, firstBlockId)).$(
'.blocklyPath',
);
await block.click();
await clickBlock(this.browser, this.firstBlock, {button: 1});
await this.browser.keys([Key.Backspace]);
await this.browser.pause(PAUSE_TIME);
// Undo
Expand Down
59 changes: 47 additions & 12 deletions tests/browser/test/test_setup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,50 @@ async function getBlockElementById(browser, id) {
return elem;
}

/**
* 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.
* @param clickOptions The options to pass to webdriverio's element.click function.
* @return A Promise that resolves when the actions are completed.
*/
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, 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 = findableId;
},
block.id,
findableId,
);

// 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);
}

/**
* @param browser The active WebdriverIO Browser object.
* @param categoryName The name of the toolbox category to find.
Expand Down Expand Up @@ -428,23 +472,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 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, 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');

// 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();
Expand Down Expand Up @@ -515,6 +549,7 @@ module.exports = {
getSelectedBlockElement,
getSelectedBlockId,
getBlockElementById,
clickBlock,
getCategory,
getNthBlockOfCategory,
getBlockTypeFromCategory,
Expand Down

0 comments on commit 00d870e

Please sign in to comment.