Repository: wix/Detox (js)

Issue:
[feature] Element Screenshot
**Is your feature request related to a problem? Please describe.**
Recently we used screenshot tests with detox to detect breaking UI changes in our components. 
In order to do that we compared screenshots of the entire screen that showcase the component. 
Unfortunately, our showcase screens contain other elements other than the tested component (titles, controls, etc..). 
If something has changed with those elements the screenshot test failed. 
It turned those tests to be very flaky so we turned them off for now. 


**Describe the solution you'd like**
A great solution will be to capture a screenshot of a specific element (by testID), this will help isolate to test case and prevent flakiness. 

**Describe alternatives you've considered**
none. 

**Additional context**
I discussed this some time ago with @amitdwix and @rotemmiz, I understand this is applicable. 
Would be very beneficial for UI testing. 


Pull request description:
Store element-screenshot results as artifacts
- [x] This is a small change 
- [x] This change has been discussed in issue #2012 and the solution has been agreed upon with maintainers.

---

**Description:**
Resolves #2012. Applies the final piece of this puzzle: the administration of artifacts management we have over image-files generated by using this feature.

Patch:
diff --git a/detox/src/Detox.js b/detox/src/Detox.js
index ffcd8bf91..cab2ee80e 100644
--- a/detox/src/Detox.js
+++ b/detox/src/Detox.js
@@ -167,6 +167,7 @@ class Detox {
 
     const matchers = matchersRegistry.resolve(this.device, {
       invocationManager,
+      emitter: this._eventEmitter,
     });
     Object.assign(this, matchers);
 
diff --git a/detox/src/android/expect.js b/detox/src/android/expect.js
index fde56a33b..dbfdad1b9 100644
--- a/detox/src/android/expect.js
+++ b/detox/src/android/expect.js
@@ -1,5 +1,6 @@
-const tempfile = require('tempfile');
 const fs = require('fs-extra');
+const path = require('path');
+const tempfile = require('tempfile');
 const invoke = require('../invoke');
 const matchers = require('./matcher');
 const DetoxActionApi = require('./espressoapi/DetoxAction');
@@ -205,8 +206,9 @@ class WaitForActionInteraction extends WaitForActionInteractionBase {
 }
 
 class Element {
-  constructor(invocationManager, matcher) {
+  constructor(invocationManager, emitter, matcher) {
     this._invocationManager = invocationManager;
+    this._emitter = emitter;
     this._originalMatcher = matcher;
     this._selectElementWithMatcher(this._originalMatcher);
   }
@@ -279,11 +281,17 @@ class Element {
     return await new ActionInteraction(this._invocationManager, this, new SwipeAction(direction, speed, percentage)).execute();
   }
 
-  async takeScreenshot() {
+  async takeScreenshot(screenshotName) {
     // TODO this should be moved to a lower-layer handler of this use-case
     const resultBase64 = await new ActionInteraction(this._invocationManager, this, new TakeElementScreenshot()).execute();
     const filePath = tempfile('detox.element-screenshot.png');
     await fs.writeFile(filePath, resultBase64, 'base64');
+
+    await this._emitter.emit('createExternalArtifact', {
+      pluginId: 'screenshot',
+      artifactName: screenshotName || path.basename(filePath, '.png'),
+      artifactPath: filePath,
+    });
     return filePath;
   }
 }
@@ -421,8 +429,9 @@ class WaitForElement extends WaitFor {
 }
 
 class AndroidExpect {
-  constructor({ invocationManager }) {
+  constructor({ invocationManager, emitter }) {
     this._invocationManager = invocationManager;
+    this._emitter = emitter;
 
     this.by = {
       accessibilityLabel: value => new LabelMatcher(value),
@@ -440,7 +449,7 @@ class AndroidExpect {
   }
 
   element(matcher) {
-    return new Element(this._invocationManager, matcher);
+    return new Element(this._invocationManager, this._emitter, matcher);
   }
 
   expect(element) {
diff --git a/detox/src/configuration/collectCliConfig.js b/detox/src/configuration/collectCliConfig.js
index 0e3898575..fde6a9dbe 100644
--- a/detox/src/configuration/collectCliConfig.js
+++ b/detox/src/configuration/collectCliConfig.js
@@ -29,6 +29,7 @@ function collectCliConfig({ argv }) {
     runnerConfig: get('runner-config'),
     useCustomLogger: get('use-custom-logger'),
     workers: get('workers'),
+    inspectBrk: get('inspect-brk'),
   }, _.isUndefined);
 }
 
diff --git a/detox/src/devices/Device.js b/detox/src/devices/Device.js
index 48450e8d9..1e6649225 100644
--- a/detox/src/devices/Device.js
+++ b/detox/src/devices/Device.js
@@ -1,6 +1,5 @@
 const _ = require('lodash');
 const debug = require('../utils/debug'); // debug utils, leave here even if unused
-const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
 
 class Device {
   constructor({ deviceConfig, deviceDriver, emitter, sessionConfig }) {
diff --git a/detox/src/devices/drivers/android/AndroidDriver.js b/detox/src/devices/drivers/android/AndroidDriver.js
index 8cbcdad65..1757280d9 100644
--- a/detox/src/devices/drivers/android/AndroidDriver.js
+++ b/detox/src/devices/drivers/android/AndroidDriver.js
@@ -1,6 +1,6 @@
+const path = require('path');
 const fs = require('fs');
 const URL = require('url').URL;
-const _ = require('lodash');
 const DeviceDriverBase = require('../DeviceDriverBase');
 const logger = require('../../../utils/logger');
 const log = logger.child({ __filename });
@@ -175,7 +175,7 @@ class AndroidDriver extends DeviceDriverBase {
 
     await this.emitter.emit('createExternalArtifact', {
       pluginId: 'screenshot',
-      artifactName: screenshotName,
+      artifactName: screenshotName || path.basename(tempPath, '.png'),
       artifactPath: tempPath,
     });
 
diff --git a/detox/src/devices/drivers/ios/SimulatorDriver.js b/detox/src/devices/drivers/ios/SimulatorDriver.js
index 083dfe286..b8f8a1f64 100644
--- a/detox/src/devices/drivers/ios/SimulatorDriver.js
+++ b/detox/src/devices/drivers/ios/SimulatorDriver.js
@@ -175,7 +175,7 @@ class SimulatorDriver extends IosDriver {
 
     await this.emitter.emit('createExternalArtifact', {
       pluginId: 'screenshot',
-      artifactName: screenshotName,
+      artifactName: screenshotName || path.basename(tempPath, '.png'),
       artifactPath: tempPath,
     });
 
diff --git a/detox/src/ios/expectTwo.js b/detox/src/ios/expectTwo.js
index f308ec1d5..789e44e4f 100644
--- a/detox/src/ios/expectTwo.js
+++ b/detox/src/ios/expectTwo.js
@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
 
 class Expect {
   constructor(element) {
@@ -197,6 +198,11 @@ class Element {
     if (!(typeof position === 'number' && position >= 0 && position <= 1)) throw new Error('position should be a number [0.0, 1.0], but got ' + (position + (' (' + (typeof position + ')'))));
     return this.withAction('adjustSliderToPosition', position);
   }
+
+  takeScreenshot() {
+    throw new DetoxRuntimeError({message: 'Element screenshots are not supported on iOS, at the moment!'});
+  }
+
   createInvocation(action, ...params) {
     return ({
       type: 'action',
diff --git a/docs/APIRef.ActionsOnElement.md b/docs/APIRef.ActionsOnElement.md
index 3d008a58e..7155c43f3 100644
--- a/docs/APIRef.ActionsOnElement.md
+++ b/docs/APIRef.ActionsOnElement.md
@@ -23,7 +23,7 @@ Use [expectations](APIRef.Expect.md) to verify element states.
 - [`.setDatePickerDate()`](#setdatepickerdatedatestring-dateformat--ios-only) **iOS only**
 - [`.adjustSliderToPosition()`](#adjustslidertopositionnormalizedposition--ios-only) **iOS only**
 - [`.getAttributes()`](#getAttributes--ios-only) **iOS only**
-- [`.takeScreenshot()`](#takescreenshot-android-only) **Android only**
+- [`.takeScreenshot([name])`](#takescreenshotname-android-only) **Android only**
 
 ### `tap(point)`
 
@@ -242,12 +242,16 @@ jestExpect(multipleMatchedElements.elements.length).toBe(5);
 jestExpect(multipleMatchedElements.elements[0].identifier).toBe('FirstElement');
 ```
 
-### `takeScreenshot()` Android Only
+### `takeScreenshot([name])` Android Only
 
-Take a screenshot of the native view associated with the element in question. Useful for highly-focused visual comparison tests (i.e. comparison between elements rather than of complete screen, with visual "noise").
+Takes a screenshot of the native view associated with the element in question, and schedules putting it to the [artifacts folder](https://github.com/wix/Detox/blob/master/docs/APIRef.Artifacts.md#enabling-artifacts) upon completion of the current test. Useful for highly-focused visual comparison tests (i.e. comparison between elements rather than of [complete screens](https://github.com/wix/Detox/blob/master/docs/APIRef.DeviceObjectAPI.md#devicetakescreenshotname) with "visual noise").
+
+`name (optional)` - Name of the final image-file to store as the artifact. For example, `my-text-field` would result in a file named `my-text-field.png`. In case the name isn't provided, Detox would self-generate a random distinct name, instead (though not a very descriptive one).
 
 Returns the path of a temporary file containing the resulted `.png` image.
 
+**NOTE:** The returned path is guaranteed to be valid only during the test execution. Later on, the screenshot will be moved to the artifacts location.
+
 ```js
 const bitmapPath = await element(by.id('fancy-element')).takeScreenshot();
 ```
@@ -263,12 +267,11 @@ if (!bitmapBuffer.equals(expectedBitmapBuffer)) {
 }
 ```
 
-> A more useful way of doing this is by utilizing more advanced 3rd-party image snapshotting and comparison tools such as [applitools](https://applitools.com).
+> The recommended, more practical way of doing this, is by utilizing more advanced 3rd-party image snapshotting & comparison tools such as [applitools](https://applitools.com).
 
-Caveats:
+**Artifacts management**
 
-* Currently, the image-file generated by Detox is stored in a temporary folder, and does not get automatically cleaned-up.
-* The image-file does not get exported as a test artifact in the test's finalization (i.e. such as with [device.takeScreenshot(name)](https://github.com/wix/Detox/blob/master/docs/APIRef.DeviceObjectAPI.md#devicetakescreenshotname)).
+Adheres to the artifacts management rules of the equivalent device-level API, [device.takeScreenshot(name)](https://github.com/wix/Detox/blob/master/docs/APIRef.DeviceObjectAPI.md#devicetakescreenshotname).
 
 ## Deprecated Methods
 
diff --git a/docs/APIRef.DetoxCLI.md b/docs/APIRef.DetoxCLI.md
index bb37b685e..8a337f321 100644
--- a/docs/APIRef.DetoxCLI.md
+++ b/docs/APIRef.DetoxCLI.md
@@ -87,6 +87,7 @@ Initiating your test suite. <sup>[[1]](#notice-passthrough)</sup>
 | --no-color                                    | Disable colors in log output |
 | --use-custom-logger | Use Detox' custom console-logging implementation, for logging Detox (non-device) logs. Disabling will fallback to node.js / test-runner's implementation (e.g. Jest / Mocha).<br />*Default: true* |
 | --force-adb-install | Due to problems with the `adb install` command on Android, Detox resorts to a different scheme for install APK's. Setting true will disable that and force usage of `adb install`, instead.<br/>This flag is temporary until the Detox way proves stable.<br/>*Default: false* |
+| --inspect-brk | Uses [node's --inspect-brk](https://nodejs.org/en/docs/guides/debugging-getting-started/#enable-inspector) flag to let users debug the jest/mocha test runner <br />*Default: false* |
 | --help                                        | Show help |
 
 ##### Notices
diff --git a/docs/APIRef.DeviceObjectAPI.md b/docs/APIRef.DeviceObjectAPI.md
index d046def40..35445c008 100644
--- a/docs/APIRef.DeviceObjectAPI.md
+++ b/docs/APIRef.DeviceObjectAPI.md
@@ -37,7 +37,7 @@ The value will be `undefined` until the device is properly _prepared_ (i.e. in `
 - [`device.disableSynchronization()`](#devicedisablesynchronization)
 - [`device.resetContentAndSettings()` **iOS Only**](#deviceresetcontentandsettings-ios-only)
 - [`device.getPlatform()`](#devicegetplatform)
-- [`device.takeScreenshot(name)`](#devicetakescreenshotname)
+- [`device.takeScreenshot([name])`](#devicetakescreenshotname)
 - [`device.shake()` **iOS Only**](#deviceshake-ios-only)
 - [`device.setBiometricEnrollment(bool)` **iOS Only**](#devicesetbiometricenrollmentbool-ios-only)
 - [`device.matchFace()` **iOS Only**](#devicematchface-ios-only)
@@ -337,16 +337,16 @@ if (device.getPlatform() === 'ios') {
 }
 ```
 
-### `device.takeScreenshot(name)`
+### `device.takeScreenshot([name])`
 
-Takes a screenshot on the device and schedules putting it to
-the [artifacts folder](APIRef.Artifacts.md#enabling-artifacts) upon
+Takes a screenshot on the device and schedules putting it tothe [artifacts folder](APIRef.Artifacts.md#enabling-artifacts) upon
 completion of the current test.
 
-Returns a temporary path to the screenshot.
+`name (optional)` - Name of the final image-file to store as the artifact. For example, `my-text-field` would result in a file named `my-text-field.png`. In case the name isn't provided, Detox would self-generate a random, distinct name, instead (though not a very descriptive one).
 
-**NOTE:** The returned path is guaranteed to be valid only during the test execution.
-Later on, the screenshot will be moved to the artifacts location.
+Returns the path of a temporary file containing the resulted `.png` image.
+
+**NOTE:** The returned path is guaranteed to be valid only during the test execution. Later on, the screenshot will be moved to the artifacts location.
 
 Consider the example below:
 


Test patch:
diff --git a/detox/local-cli/test.js b/detox/local-cli/test.js
index 0cdc72471..0ef3a5963 100644
--- a/detox/local-cli/test.js
+++ b/detox/local-cli/test.js
@@ -142,6 +142,10 @@ module.exports.builder = {
     group: 'Execution:',
     describe: `Due to problems with the "adb install" command on Android, Detox resorts to a different scheme for install APK's. Setting true will disable that and force usage of "adb install", instead.`,
   },
+  'inspect-brk': {
+    group: 'Debugging:',
+    describe: 'Allows debugging of the underlying test runner',
+  }
 };
 
 const collectExtraArgs = require('./utils/collectExtraArgs')(module.exports.builder);
@@ -199,6 +203,7 @@ module.exports.handler = async function test(program) {
       : 'config';
 
     const command = _.compact([
+      cliConfig.inspectBrk ? 'node --inspect-brk' : '',
       (path.join('node_modules', '.bin', runnerConfig.testRunner)),
       ...safeGuardArguments([
         (runnerConfig.runnerConfig ? `--${configParam} ${runnerConfig.runnerConfig}` : ''),
@@ -251,6 +256,7 @@ module.exports.handler = async function test(program) {
     }
 
     const command = _.compact([
+      cliConfig.inspectBrk ? 'node --inspect-brk' : '',
       path.join('node_modules', '.bin', runnerConfig.testRunner),
       ...safeGuardArguments([
         cliConfig.noColor ? ' --no-color' : '',
diff --git a/detox/src/Detox.test.js b/detox/src/Detox.test.js
index 4b3bd4b95..db2a54398 100644
--- a/detox/src/Detox.test.js
+++ b/detox/src/Detox.test.js
@@ -1,4 +1,3 @@
-const _ = require('lodash');
 const configuration = require('./configuration');
 const testSummaries = require('./artifacts/__mocks__/testSummaries.mock');
 
@@ -30,17 +29,10 @@ describe('Detox', () => {
   let detox;
   let lifecycleSymbols;
 
-  function client() {
-    return Client.mock.instances[0];
-  }
-
-  function device() {
-    return Device.mock.instances[0];
-  }
-
-  function artifactsManager() {
-    return ArtifactsManager.mock.instances[0];
-  }
+  const client = () => Client.mock.instances[0];
+  const device = () => Device.mock.instances[0];
+  const artifactsManager = () => ArtifactsManager.mock.instances[0];
+  const invocationManager = () => invoke.InvocationManager.mock.instances[0];
 
   beforeEach(async () => {
     detoxConfig = await configuration.composeDetoxConfig({
@@ -117,15 +109,18 @@ describe('Detox', () => {
           detoxConfig.deviceConfig.type,
           {
             client: client(),
-            invocationManager: invoke.InvocationManager.mock.instances[0],
+            invocationManager: invocationManager(),
             emitter: expect.anything(),
           }
         ));
 
-      it('should resolve matchers implementation', () =>
+      it('should resolve matchers implementation', () => {
+        const { emitter } = Device.mock.calls[0][0];
         expect(matchersRegistry.resolve).toHaveBeenCalledWith(device(), {
-          invocationManager: invoke.InvocationManager.mock.instances[0],
-        }));
+          invocationManager: invocationManager(),
+          emitter,
+        });
+      });
 
       it('should take the matchers from the matchers-registry to Detox', () =>
         expect(detox.globalMatcher).toBe(mockGlobalMatcher));
diff --git a/detox/src/android/expect.test.js b/detox/src/android/expect.test.js
index e110b6712..7cfbecfa7 100644
--- a/detox/src/android/expect.test.js
+++ b/detox/src/android/expect.test.js
@@ -2,15 +2,20 @@ describe('expect', () => {
   let e;
 
   let mockExecutor;
+  let emitter;
   beforeEach(() => {
     jest.mock('tempfile');
     jest.mock('fs-extra');
 
     mockExecutor = new MockExecutor();
 
+    const Emitter = jest.genMockFromModule('../utils/AsyncEmitter');
+    emitter = new Emitter();
+
     const AndroidExpect = require('./expect');
     e = new AndroidExpect({
       invocationManager: mockExecutor,
+      emitter,
     });
   });
 
@@ -210,7 +215,7 @@ describe('expect', () => {
   });
 
   describe('element screenshots', () => {
-    const tempFilePath = '/path/to/temp-file.ext';
+    const tempFilePath = '/path/to/temp-file.png';
     const invokeResultInBase64 = 'VGhlcmUgaXMgbm8gc3Bvb24h';
 
     let tempfile;
@@ -236,6 +241,26 @@ describe('expect', () => {
       const result = await _element.takeScreenshot();
       expect(result).toEqual(tempFilePath);
     });
+
+    it('should emit a named-artifact creation event', async () => {
+      const screenshotName = 'mock-screenshot-name';
+      await _element.takeScreenshot(screenshotName);
+      expect(emitter.emit).toHaveBeenCalledWith('createExternalArtifact', {
+        pluginId: 'screenshot',
+        artifactName: screenshotName,
+        artifactPath: tempFilePath,
+      });
+    });
+
+    it('should emit an artifact creation event with a default name', async () => {
+      await _element.takeScreenshot(undefined);
+      expect(emitter.emit).toHaveBeenCalledWith(
+        expect.any(String),
+        expect.objectContaining({
+          artifactName: 'temp-file',
+        },
+      ));
+    });
   });
 });
 
diff --git a/detox/src/configuration/collectCliConfig.test.js b/detox/src/configuration/collectCliConfig.test.js
index 43c9aaa06..9b5b33294 100644
--- a/detox/src/configuration/collectCliConfig.test.js
+++ b/detox/src/configuration/collectCliConfig.test.js
@@ -38,6 +38,7 @@ describe('collectCliConfig', () => {
     ['runnerConfig', 'runner-config'],
     ['useCustomLogger', 'use-custom-logger'],
     ['workers', 'workers'],
+    ['inspectBrk', 'inspect-brk'],
   ])('.%s property', (key, argName) => {
     beforeEach(() => {
       argv[argName] = Math.random();
diff --git a/detox/src/ios/expectTwo.test.js b/detox/src/ios/expectTwo.test.js
index b88651bfb..2dd87623c 100644
--- a/detox/src/ios/expectTwo.test.js
+++ b/detox/src/ios/expectTwo.test.js
@@ -361,6 +361,15 @@ describe('expectTwo', () => {
 
     expect(testCall).deepEquals(jsonOutput);
   });
+
+  it(`element().takeScreenshot`, () => {
+    try {
+      e.element(e.by.id('uniqueId')).takeScreenshot();
+      fail();
+    } catch (e) {
+      expect(e.message).toContain('not supported on iOS');
+    }
+  });
 });
 
 expect.extend({
diff --git a/detox/test/e2e/26.element-screenshots.test.js b/detox/test/e2e/26.element-screenshots.test.js
index 6048f5ce3..f90c1b011 100644
--- a/detox/test/e2e/26.element-screenshots.test.js
+++ b/detox/test/e2e/26.element-screenshots.test.js
@@ -20,7 +20,7 @@ describe(':android: Element screenshots', () => {
 
     await element(by.id('switchOrientation')).tap();
 
-    const bitmapPath = await element(by.id('fancyElement')).takeScreenshot();
+    const bitmapPath = await element(by.id('fancyElement')).takeScreenshot('fancy-element');
     expectBitmapsToBeEqual(bitmapPath, screenshotAssetPath);
   });