fix: DEV-4081: e2e tests for selected region after tool changes (#1192)

* e2e tests to ensure selected region is correct after moving between certain tools.

When using Brush then Eraser or Pan, existing region should stay selected and have new additions be added to it. Similar for the Magic Wand.

We also have to ensure that the unselectRegionOnToolChange property for the Brush tool is updated on `afterCreate` so that e2e tests can dynamically update a feature flag and have it be applied.

* Make sure common helper function is available.

* Small code style fixes.

* Ensure magicwand tag is feature flagged on imports, not dynamically in the Registry.

This is safer for side effects. We've also modified our e2e tests to work with this different feature flagging approach.

* Make changes based on code review feedback.

---------

Co-authored-by: bradneuberg <[email protected]>
Co-authored-by: hlomzik <[email protected]>

Author: 3 people

e2e/tests/helpers.js

@@ -1,3 +1,5 @@
+const assert = require('assert');
+
 /**
  * Load custom example
  * @param {object} params
@@ -823,6 +825,16 @@ function hasSelectedRegion() {
   return !!Htx.annotationStore.selected.highlightedNode;
 }
 
+async function doDrawingAction(I, { msg, fromX, fromY, toX, toY }) {
+  I.usePlaywrightTo(msg, async ({ browser, browserContext, page }) => {
+    await page.mouse.move(fromX, fromY);
+    await page.mouse.down();
+    await page.mouse.move(toX, toY);
+    await page.mouse.up();
+  });
+  I.wait(1); // Ensure that the tool is fully finished being created.
+}
+
 // `mulberry32` (simple generator with a 32-bit state)
 function createRandomWithSeed(seed) {
   return function() {
@@ -833,6 +845,7 @@ function createRandomWithSeed(seed) {
     return ((t ^ t >>> 14) >>> 0) / 4294967296;
   };
 }
+
 function createRandomIntWithSeed(seed) {
   const random = createRandomWithSeed(seed);
 
@@ -894,6 +907,7 @@ module.exports = {
   omitBy,
   dumpJSON,
 
+  doDrawingAction,
   createRandomWithSeed,
   createRandomIntWithSeed,
 };

e2e/tests/image.magic-wand.test.js

@@ -1,5 +1,6 @@
 const {
   initLabelStudio,
+  doDrawingAction,
   hasKonvaPixelColorAtPoint,
   setKonvaLayersOpacity,
   serialize,
@@ -41,8 +42,6 @@ const annotationEmpty = {
   result: [],
 };
 
-// TODO: Change these URLs to heartex URLs, and ensure the heartex bucket allows CORS access
-// for these to work.
 const data = {
   'image': [
     'http://htx-pub.s3.amazonaws.com/samples/magicwand/magic_wand_scale_1_20200902_015806_26_2235_1B_AnalyticMS_00750_00750.jpg',
@@ -53,16 +52,6 @@ const data = {
   'thumb': 'http://htx-pub.s3.amazonaws.com/samples/magicwand/magic_wand_thumbnail_20200902_015806_26_2235_1B_AnalyticMS_00750_00750.jpg',
 };
 
-async function magicWand(I, { msg, fromX, fromY, toX, toY }) {
-  I.usePlaywrightTo(msg, async ({ page }) => {
-    await page.mouse.move(fromX, fromY);
-    await page.mouse.down();
-    await page.mouse.move(toX, toY);
-    await page.mouse.up();
-  });
-  I.wait(1); // Ensure that the magic wand brush region is fully finished being created.
-}
-
 async function assertMagicWandPixel(I, x, y, assertValue, rgbArray, msg) {
   const hasPixel = await I.executeScript(hasKonvaPixelColorAtPoint, [x, y, rgbArray, 1]);
 
@@ -100,8 +89,8 @@ Scenario('Make sure the magic wand works in a variety of scenarios', async funct
   AtSidebar.seeRegions(0);
 
   I.say('Magic wanding clouds with cloud class in upper left of image');
-  await magicWand(I, { msg: 'Fill in clouds upper left', fromX: 258, fromY: 214, toX: 650, toY: 650 });
-  await magicWand(I, { msg: 'Fill in clouds lower left', fromX: 337, fromY: 777, toX: 650, toY: 650 });
+  await doDrawingAction(I, { msg: 'Fill in clouds upper left', fromX: 258, fromY: 214, toX: 650, toY: 650 });
+  await doDrawingAction(I, { msg: 'Fill in clouds lower left', fromX: 337, fromY: 777, toX: 650, toY: 650 });
 
   I.say('Ensuring repeated magic wands back to back with same class collapsed into single region');
   AtSidebar.seeRegions(1);
@@ -172,7 +161,7 @@ Scenario('Make sure the magic wand works in a variety of scenarios', async funct
   I.pressKey('2');
 
   I.say('Magic wanding cloud shadows with cloud shadow class in center of zoomed image');
-  await magicWand(I, { msg: 'Cloud shadow in middle of image', fromX: 390, fromY: 500, toX: 500, toY: 500 });
+  await doDrawingAction(I, { msg: 'Cloud shadow in middle of image', fromX: 390, fromY: 500, toX: 500, toY: 500 });
 
   I.say('Ensuring new cloud shadow magic wand region gets added to sidebar');
   AtSidebar.seeRegions(2);

e2e/tests/image.selected-region.test.js

@@ -0,0 +1,110 @@
+/* global Feature, Scenario */
+
+const {
+  doDrawingAction,
+  initLabelStudio,
+  waitForImage,
+} = require('./helpers');
+const assert = require('assert');
+
+Feature('Test Image Region Stay Selected Between Tools');
+
+const PLANET = {
+  color: '#00FF00',
+  rgbArray: [0, 255, 0],
+};
+const MOONWALKER = {
+  color: '#0000FF',
+  rgbArray: [0, 0, 255],
+};
+
+const config = `
+  <View>
+    <Image name="image" value="$image" crossOrigin="anonymous" />
+    <Brush name="brush" toName="image" />
+    <MagicWand name="magicwand" toName="image" />
+    <Labels name="labels" toName="image" fillOpacity="0.5" strokeWidth="5">
+      <Label value="Planet" background="${PLANET.color}"></Label>
+      <Label value="Moonwalker" background="${MOONWALKER.color}"></Label>
+    </Labels>
+  </View>`;
+
+const annotationEmpty = {
+  id: '1000',
+  result: [],
+};
+
+const data = {
+  image:
+    'https://htx-misc.s3.amazonaws.com/opensource/label-studio/examples/images/nick-owuor-astro-nic-visuals-wDifg5xc9Z4-unsplash.jpg',
+};
+
+async function testRegion(testType, toolAccelerator, I, LabelStudio, AtImageView, AtSidebar) {
+  const params = {
+    config,
+    data,
+    annotations: [annotationEmpty],
+  };
+
+  LabelStudio.setFeatureFlags({
+    'fflag_feat_front_dev_4081_magic_wand_tool': true,
+  });
+
+  I.amOnPage('/');
+
+  I.executeScript(initLabelStudio, params);
+
+  AtImageView.waitForImage();
+  await AtImageView.lookForStage();
+  I.executeScript(waitForImage);
+
+  I.say(`Select ${testType} & planet class`);
+  I.pressKey(toolAccelerator);
+  I.pressKey('1');
+
+  I.say('There should be no regions initially');
+  AtSidebar.seeRegions(0);
+
+  I.say(`${testType} initial region`);
+  await doDrawingAction(I, { msg: `Initial ${testType}`, fromX: 150, fromY: 110, toX: 150+50, toY: 110+50 });
+
+  I.say('There should now be a single region');
+  AtSidebar.seeRegions(1);
+
+  I.say(`Using Eraser on ${testType} region`);
+  I.pressKey('E');
+  I.usePlaywrightTo('Erasing', async ({ browser, browserContext, page }) => {
+    await page.mouse.move(150, 150);
+    await page.mouse.down();
+    await page.mouse.move(150+100, 150);
+    await page.mouse.up();
+  });
+
+  I.say(`Doing another ${testType} with same class after erasing`);
+  I.pressKey(toolAccelerator);
+  await doDrawingAction(I, { msg: `${testType} after erasing`, fromX: 280, fromY: 480, toX: 280+50, toY: 480+50 });
+
+  I.say('There should still only be one region');
+  AtSidebar.seeRegions(1);
+
+  I.say('Zooming and selecting pan tool');
+  I.click('button[aria-label="zoom-in"]');
+  I.click('button[aria-label="zoom-in"]');
+  I.pressKey('H');
+
+  I.say(`Doing another ${testType} after zooming and selecting pan tool`);
+  I.pressKey(toolAccelerator);
+  await doDrawingAction(I, { msg: `${testType} after zoom and pan selected`,
+    fromX: 400, fromY: 200, toX: 400+15, toY: 400+15 });
+
+  I.say('There should still only be one region');
+  AtSidebar.seeRegions(1);
+}
+
+Scenario('Selected brush region should stay between tools', async function({ I, LabelStudio, AtImageView, AtSidebar }) {
+  await testRegion('brush', 'B', I, LabelStudio, AtImageView, AtSidebar);
+});
+
+Scenario('Selected Magic Wand region should stay between tools', async function({ I, LabelStudio, AtImageView, AtSidebar }) {
+  await testRegion('magicwand', 'W', I, LabelStudio, AtImageView, AtSidebar);
+});

src/tools/Brush.js

@@ -53,6 +53,9 @@ const _Tool = types
     group: 'segmentation',
     shortcut: 'B',
     smart: true,
+
+    // Support the existing unselect behavior until the Magic Wand feature flag is on by default.
+    // @todo change to false once the Magic Wand is on by default.
     unselectRegionOnToolChange: isFF(FF_DEV_4081) ? false : true,
   })
   .volatile(() => ({