refactor: apply code review changes.

detox/src/android/actions/native.js

@@ -25,10 +25,10 @@ class TapAtPointAction extends Action {
 }
 
 class LongPressAction extends Action {
-  constructor(x, y, duration) {
+  constructor(point, duration) {
     super();
 
-    const filteredArgs = [x, y, duration].filter((arg) => arg !== null && arg !== undefined);
+    const filteredArgs = (point ? [point.x, point.y] : []).concat(duration ? [duration] : []);
     this._call = invoke.callDirectly(DetoxActionApi.longPress(...filteredArgs));
   }
 }

detox/src/android/core/NativeElement.js

@@ -7,6 +7,7 @@ const DetoxRuntimeError = require('../../errors/DetoxRuntimeError');
 const invoke = require('../../invoke');
 const { removeMilliseconds } = require('../../utils/dateUtils');
 const { actionDescription } = require('../../utils/invocationTraceDescriptions');
+const mapLongPressArguments = require('../../utils/mapLongPressArguments');
 const actions = require('../actions/native');
 const DetoxMatcherApi = require('../espressoapi/DetoxMatcher');
 const { ActionInteraction } = require('../interactions/native');
@@ -42,31 +43,14 @@ class NativeElement {
     return await new ActionInteraction(this._invocationManager, this._matcher, action, traceDescription).execute();
   }
 
-  async longPress(arg1, arg2) {
-    let point = null;
-    let duration = null;
+  async longPress(optionalPointOrDuration, optionalDuration) {
+    const { point, duration } = mapLongPressArguments(optionalPointOrDuration, optionalDuration);
 
-    if (arg1 !== undefined && typeof arg1 === 'number' && arg2 === undefined) {
-      duration = arg1;
-    } else if (arg1 !== undefined && this._checkIfPointIsValid(arg1) && arg2 === undefined) {
-      point = arg1;
-    } else if (arg1 !== undefined && typeof arg1 === 'number' && this._checkIfPointIsValid(arg2)) {
-      duration = arg1;
-      point = arg2;
-    } else if (arg1 !== undefined || arg2 !== undefined) {
-      throw new Error('longPress accepts either a duration (number) or a point ({x: number, y: number}) as ' +
-          'its first argument, and optionally a point as its second argument (if the first argument is a duration).');
-    }
-
-    const action = new actions.LongPressAction(point && point.x, point && point.y, duration);
+    const action = new actions.LongPressAction(point, duration);
     const traceDescription = actionDescription.longPress(point, duration);
     return await new ActionInteraction(this._invocationManager, this._matcher, action, traceDescription).execute();
   }
 
-  _checkIfPointIsValid(point) {
-    return typeof point === 'object' && typeof point.x === 'number' && typeof point.y === 'number';
-  }
-
   async longPressAndDrag(duration, normalizedPositionX, normalizedPositionY, targetElement, normalizedTargetPositionX, normalizedTargetPositionY, speed, holdDuration) {
     const action = new actions.LongPressAndDragAction(
       duration,

detox/src/ios/expectTwo.js

@@ -8,10 +8,10 @@ const tempfile = require('tempfile');
 
 
 const { assertEnum, assertNormalized } = require('../utils/assertArgument');
-const { removeMilliseconds } = require('../utils/dateUtils');
 const { actionDescription, expectDescription } = require('../utils/invocationTraceDescriptions');
 const { isRegExp } = require('../utils/isRegExp');
 const log = require('../utils/logger').child({ cat: 'ws-client, ws' });
+const mapLongPressArguments = require('../utils/mapLongPressArguments');
 const traceInvocationCall = require('../utils/traceInvocationCall').bind(null, log);
 
 const { webElement, webMatcher, webExpect, isWebElement } = require('./web');
@@ -162,7 +162,8 @@ class Element {
   }
 
   longPress(arg1, arg2) {
-    let { point, duration } = _longPressPointAndDuration(arg1, arg2);
+    let { point, duration } = mapLongPressArguments(arg1, arg2);
+
     const traceDescription = actionDescription.longPress(point, duration);
     return this.withAction('longPress', traceDescription, point, duration);
   }
@@ -593,7 +594,7 @@ class WaitFor {
   longPress(arg1, arg2) {
     this.action = this.actionableElement.longPress(arg1, arg2);
 
-    let { point, duration } = _longPressPointAndDuration(arg1, arg2);
+    let { point, duration } = mapLongPressArguments(arg1, arg2);
     const traceDescription = actionDescription.longPress(point, duration);
 
     return this.waitForWithAction(traceDescription);
@@ -798,26 +799,6 @@ class IosExpect {
   }
 }
 
-function _longPressPointAndDuration(arg1, arg2) {
-  let point = null;
-  let duration = null;
-
-  if (typeof arg1 === 'number' || arg1 === undefined) {
-    duration = arg1;
-
-    _assertValidPoint(arg2);
-    point = arg2;
-  } else if (typeof arg1 === 'object' && arg2 === undefined) {
-    _assertValidPoint(arg1);
-    point = arg1;
-  } else {
-    throw new Error('longPress accepts either a duration (number) or a point ({x: number, y: number}) ' +
-        'as its first argument, and optionally a point as its second argument (if the first argument is a duration).');
-  }
-
-  return { point, duration };
-}
-
 function _assertValidPoint(point) {
   if (!point) {
     // point is optional

detox/src/utils/assertArgument.js

@@ -36,9 +36,27 @@ function assertEnum(allowedValues) {
   };
 }
 
+function assertDuration(duration) {
+  if (typeof duration === 'number') {
+    return true;
+  }
+
+  throw new DetoxRuntimeError('duration should be a number, but got ' + (duration + (' (' + (typeof duration + ')'))));
+}
+
+function assertPoint(point) {
+  if (typeof point === 'object' && typeof point.x === 'number' &&  typeof point.y === 'number') {
+    return true;
+  }
+
+  throw new DetoxRuntimeError(`point should be an object with x and y properties, but got ${JSON.stringify(point)}`);
+}
+
 module.exports = {
   assertEnum,
   assertNormalized,
   assertNumber,
   assertString,
+  assertDuration,
+  assertPoint,
 };

detox/src/utils/assertArgument.test.js

@@ -73,3 +73,41 @@ describe('assertString', () => {
     expect(() => assertString({ invalidString })).toThrowErrorMatchingSnapshot();
   });
 });
+
+describe('assertDuration', () => {
+  const { assertDuration } = assertions;
+
+  it.each([
+    42,
+    NaN,
+    Infinity,
+    -Infinity,
+  ])('should pass for %d', (validNumber) => {
+    expect(() => assertDuration(validNumber)).toBe(true);
+  });
+
+  it.each([
+    '42',
+    false,
+  ])('should throw for %j', (invalidNumber) => {
+    expect(() => assertDuration(invalidNumber)).toThrowErrorMatchingSnapshot();
+  });
+});
+
+describe('assertPoint', () => {
+  const { assertPoint } = assertions;
+
+  it('should pass for valid point', () => {
+    expect(() => assertPoint({ x: 0, y: 0 })).toBe(true);
+  });
+
+  it.each([
+    { x: 0 },
+    { y: 0 },
+    { x: '0', y: 0 },
+    { x: 0, y: '0' },
+    { x: 0, y: 0, z: 0 },
+  ])('should throw for %j', (invalidPoint) => {
+    expect(() => assertPoint(invalidPoint)).toThrowErrorMatchingSnapshot();
+  });
+});

detox/src/utils/mapLongPressArguments.js

@@ -0,0 +1,28 @@
+const { assertPoint, assertDuration } = require('./assertArgument');
+
+function mapLongPressArguments(optionalPointOrDuration, optionalDuration) {
+  let point = null;
+  let duration = null;
+
+  try {
+    if (optionalPointOrDuration !== undefined) {
+      assertPoint(optionalPointOrDuration);
+      point = optionalPointOrDuration;
+
+      if (optionalDuration !== undefined) {
+        assertDuration(optionalDuration);
+        duration = optionalDuration;
+      }
+    } else if (optionalDuration !== undefined) {
+      assertDuration(optionalDuration);
+      duration = optionalDuration;
+    }
+  } catch (e) {
+    throw new Error(`longPress accepts either a duration (number) or a point ({x: number, y: number}) as ` +
+      `its first argument, and optionally a duration (number) as its second argument. Error: ${e.message}`);
+  }
+
+  return { point, duration };
+}
+
+module.exports = mapLongPressArguments;

detox/src/utils/mapLongPressArguments.test.js

@@ -0,0 +1,30 @@
+const mapLongPressArguments = require('./mapLongPressArguments');
+describe('mapLongPressArguments', () => {
+  it('should return { point: { x: 1, y: 2 }, duration: 3 } for { x: 1, y: 2 }, 3', () => {
+    expect(mapLongPressArguments({ x: 1, y: 2 }, 3)).toEqual({ point: { x: 1, y: 2 }, duration: 3 });
+  });
+
+  it('should return { point: { x: 1, y: 2 }, duration: null } for { x: 1, y: 2 }', () => {
+    expect(mapLongPressArguments({ x: 1, y: 2 })).toEqual({ point: { x: 1, y: 2 }, duration: null });
+  });
+
+  it('should return { point: null, duration: 3 } for null, 3', () => {
+    expect(mapLongPressArguments(null, 3)).toEqual({ point: null, duration: 3 });
+  });
+
+  it('should return { point: null, duration: 3 } for 3', () => {
+    expect(mapLongPressArguments(3)).toEqual({ point: null, duration: 3 });
+  });
+
+  it('should return { point: null, duration: null } for no arguments', () => {
+    expect(mapLongPressArguments()).toEqual({ point: null, duration: null });
+  });
+
+  it('should throw for invalid point', () => {
+    expect(() => mapLongPressArguments({ x: 1 })).toThrowError('point should be an object with x and y properties, but got {"x":1}');
+  });
+
+  it('should throw for invalid duration', () => {
+    expect(() => mapLongPressArguments({ x: 1, y: 2 }, '3')).toThrowError('duration should be a number, but got 3 (string)');
+  });
+});