fix: appium:options should work via --default-capabilities

fix #18191
appium · Feb 14, 2023 · 11e7ad0 · 11e7ad0
1 parent c69b602
commit 11e7ad0
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 81 deletions.
diff --git a/packages/appium/lib/appium.js b/packages/appium/lib/appium.js
@@ -10,6 +10,7 @@ import {
   DELETE_SESSION_COMMAND,
   GET_STATUS_COMMAND,
   promoteAppiumOptions,
+  promoteAppiumOptionsForObject,
 } from '@appium/base-driver';
 import AsyncLock from 'async-lock';
 import {parseCapsForInnerDriver, pullSettings} from './utils';
@@ -243,7 +244,7 @@ class AppiumDriver extends DriverCore {
         jsonwpCaps,
         promoteAppiumOptions(w3cCapabilities),
         this.desiredCapConstraints,
-        defaultCapabilities
+        defaultCapabilities ? promoteAppiumOptionsForObject(defaultCapabilities) : undefined
       );
 
       const {desiredCaps, processedJsonwpCapabilities, processedW3CCapabilities} =

diff --git a/packages/appium/test/e2e/driver.e2e.spec.js b/packages/appium/test/e2e/driver.e2e.spec.js
@@ -88,35 +88,32 @@ describe('FakeDriver via HTTP', function () {
     }
 
     FakeDriver = driverConfig.require('fake');
-    // then start server if we need to
-    await serverStart(port, {appiumHome});
   });
 
   after(async function () {
-    await serverClose();
     await fs.rimraf(appiumHome);
     sandbox.restore();
   });
 
   /**
-   *
-   * @param {number} port
    * @param {Partial<import('appium/types').ParsedArgs>} [args]
    */
-  async function serverStart(port, args = {}) {
-    args = {...args, port, address: TEST_HOST};
-    if (shouldStartServer) {
-      server = await appiumServer(args);
-    }
-  }
-
-  async function serverClose() {
-    if (server) {
-      await server.close();
-    }
+  function withServer(args = {}) {
+    before(async function () {
+      args = {...args, appiumHome, port, address: TEST_HOST};
+      if (shouldStartServer) {
+        server = await appiumServer(args);
+      }
+    });
+    after(async function () {
+      if (server) {
+        await server.close();
+      }
+    });
   }
 
   describe('server updating', function () {
+    withServer();
     it('should allow drivers to update the server in arbitrary ways', async function () {
       const {data} = await axios.get(`${testServerBaseUrl}/fakedriver`);
       data.should.eql({fakedriver: 'fakeResponse'});
@@ -132,6 +129,7 @@ describe('FakeDriver via HTTP', function () {
   });
 
   describe('cli args handling for empty args', function () {
+    withServer();
     it('should not receive user cli args if none passed in', async function () {
       let driver = await wdio({...wdOpts, capabilities: caps});
       const {sessionId} = driver;
@@ -146,16 +144,7 @@ describe('FakeDriver via HTTP', function () {
   });
 
   describe('cli args handling for passed in args', function () {
-    before(async function () {
-      await serverClose();
-      await serverStart(port, {appiumHome, ...FAKE_DRIVER_ARGS});
-    });
-    after(async function () {
-      await serverClose();
-      // this weirdness here is to restart the same server which was originally started in the parent suite's
-      // "before all" hook.  another way of doing this would be to just...not nest suites this way.
-      await serverStart(port, {appiumHome});
-    });
+    withServer(FAKE_DRIVER_ARGS);
     it('should receive user cli args from a driver if arguments were passed in', async function () {
       let driver = await wdio({...wdOpts, capabilities: caps});
       const {sessionId} = driver;
@@ -169,7 +158,46 @@ describe('FakeDriver via HTTP', function () {
     });
   });
 
+  describe('default capabilities via cli', function () {
+    withServer({
+      defaultCapabilities: {
+        'appium:options': {
+          automationName: 'Fake',
+          deviceName: 'Fake',
+          app: TEST_FAKE_APP,
+        },
+        platformName: 'Fake',
+      },
+    });
+    it('should allow appium-prefixed caps sent via appium:options through --default-capabilities', async function () {
+      const appiumOptsCaps = {
+        capabilities: {
+          alwaysMatch: {},
+          firstMatch: [{}],
+        },
+      };
+
+      // Create the session
+      const {value} = (await axios.post(testServerBaseSessionUrl, appiumOptsCaps)).data;
+      try {
+        value.sessionId.should.be.a.string;
+        value.should.exist;
+        value.capabilities.should.deep.equal({
+          automationName: 'Fake',
+          platformName: 'Fake',
+          deviceName: 'Fake',
+          app: TEST_FAKE_APP,
+        });
+      } finally {
+        // End session
+        await axios.delete(`${testServerBaseSessionUrl}/${value.sessionId}`);
+      }
+    });
+  });
+
   describe('session handling', function () {
+    withServer();
+
     it('should start and stop a session and not allow commands after session stopped', async function () {
       let driver = await wdio({...wdOpts, capabilities: caps});
       should.exist(driver.sessionId);

diff --git a/packages/base-driver/lib/basedriver/capabilities.js b/packages/base-driver/lib/basedriver/capabilities.js
@@ -334,77 +334,85 @@ function processCapabilities(
 }
 
 /**
- * Return a copy of a capabilities object which has taken everything within the 'options'
- * capability and promoted it to the top level.
+ * Return a copy of a "bare" (single-level, non-W3C) capabilities object which has taken everything
+ * within the 'appium:options' capability and promoted it to the top level.
  *
  * @template {Constraints} C
- * @param {import('@appium/types').W3CCapabilities<C>} originalCaps
- * @return {import('@appium/types').W3CCapabilities<C>} the capabilities with 'options' promoted if necessary
+ * @param {NSCapabilities<C>} obj
+ * @return {NSCapabilities<C>} the capabilities with 'options' promoted if necessary
  */
-function promoteAppiumOptions(originalCaps) {
-  const promoteForObject = (obj) => {
-    const appiumOptions = obj[PREFIXED_APPIUM_OPTS_CAP];
-    if (!appiumOptions) {
-      return obj;
-    }
+function promoteAppiumOptionsForObject(obj) {
+  const appiumOptions = obj[PREFIXED_APPIUM_OPTS_CAP];
+  if (!appiumOptions) {
+    return obj;
+  }
+
+  if (!_.isPlainObject(appiumOptions)) {
+    throw new errors.SessionNotCreatedError(
+      `The ${PREFIXED_APPIUM_OPTS_CAP} capability must be an object`
+    );
+  }
+  if (_.isEmpty(appiumOptions)) {
+    return obj;
+  }
 
-    if (!_.isPlainObject(appiumOptions)) {
+  log.debug(
+    `Found ${PREFIXED_APPIUM_OPTS_CAP} capability present; will promote items inside to caps`
+  );
+
+  /**
+   * @param {string} capName
+   */
+  const shouldAddVendorPrefix = (capName) => !capName.startsWith(APPIUM_VENDOR_PREFIX);
+  const verifyIfAcceptable = (/** @type {string} */ capName) => {
+    if (!_.isString(capName)) {
       throw new errors.SessionNotCreatedError(
-        `The ${PREFIXED_APPIUM_OPTS_CAP} capability must be an object`
+        `Capability names in ${PREFIXED_APPIUM_OPTS_CAP} must be strings. '${capName}' is unexpected`
       );
     }
-    if (_.isEmpty(appiumOptions)) {
-      return obj;
-    }
-
-    log.debug(
-      `Found ${PREFIXED_APPIUM_OPTS_CAP} capability present; will promote items inside to caps`
-    );
-
-    /**
-     * @param {string} capName
-     */
-    const shouldAddVendorPrefix = (capName) => !capName.startsWith(APPIUM_VENDOR_PREFIX);
-    const verifyIfAcceptable = (capName) => {
-      if (!_.isString(capName)) {
-        throw new errors.SessionNotCreatedError(
-          `Capability names in ${PREFIXED_APPIUM_OPTS_CAP} must be strings. '${capName}' is unexpected`
-        );
-      }
-      if (isStandardCap(capName)) {
-        throw new errors.SessionNotCreatedError(
-          `${PREFIXED_APPIUM_OPTS_CAP} must only contain vendor-specific capabilties. '${capName}' is unexpected`
-        );
-      }
-      return capName;
-    };
-    const preprocessedOptions = _(appiumOptions)
-      .mapKeys((value, key) => verifyIfAcceptable(key))
-      .mapKeys((value, key) => (shouldAddVendorPrefix(key) ? `${APPIUM_VENDOR_PREFIX}${key}` : key))
-      .value();
-    // warn if we are going to overwrite any keys on the base caps object
-    const overwrittenKeys = _.intersection(Object.keys(obj), Object.keys(preprocessedOptions));
-    if (overwrittenKeys.length > 0) {
-      log.warn(
-        `Found capabilities inside ${PREFIXED_APPIUM_OPTS_CAP} that will overwrite ` +
-          `capabilities at the top level: ${JSON.stringify(overwrittenKeys)}`
+    if (isStandardCap(capName)) {
+      throw new errors.SessionNotCreatedError(
+        `${PREFIXED_APPIUM_OPTS_CAP} must only contain vendor-specific capabilties. '${capName}' is unexpected`
       );
     }
-    return _.cloneDeep({
-      ..._.omit(obj, PREFIXED_APPIUM_OPTS_CAP),
-      ...preprocessedOptions,
-    });
+    return capName;
   };
+  const preprocessedOptions = _(appiumOptions)
+    .mapKeys((value, /** @type {string} */ key) => verifyIfAcceptable(key))
+    .mapKeys((value, key) => (shouldAddVendorPrefix(key) ? `${APPIUM_VENDOR_PREFIX}${key}` : key))
+    .value();
+  // warn if we are going to overwrite any keys on the base caps object
+  const overwrittenKeys = _.intersection(Object.keys(obj), Object.keys(preprocessedOptions));
+  if (overwrittenKeys.length > 0) {
+    log.warn(
+      `Found capabilities inside ${PREFIXED_APPIUM_OPTS_CAP} that will overwrite ` +
+        `capabilities at the top level: ${JSON.stringify(overwrittenKeys)}`
+    );
+  }
+  return _.cloneDeep({
+    .../** @type {NSCapabilities<C>} */ (_.omit(obj, PREFIXED_APPIUM_OPTS_CAP)),
+    ...preprocessedOptions,
+  });
+}
 
-  const {alwaysMatch, firstMatch} = originalCaps;
+/**
+ * Return a copy of a capabilities object which has taken everything within the 'options'
+ * capability and promoted it to the top level.
+ *
+ * @template {Constraints} C
+ * @param {import('@appium/types').W3CCapabilities<C>} originalCaps
+ * @return {import('@appium/types').W3CCapabilities<C>} the capabilities with 'options' promoted if necessary
+ */
+function promoteAppiumOptions(originalCaps) {
   const result = {};
+  const {alwaysMatch, firstMatch} = originalCaps;
   if (_.isPlainObject(alwaysMatch)) {
-    result.alwaysMatch = promoteForObject(alwaysMatch);
+    result.alwaysMatch = promoteAppiumOptionsForObject(alwaysMatch);
   } else if ('alwaysMatch' in originalCaps) {
     result.alwaysMatch = alwaysMatch;
   }
   if (_.isArray(firstMatch)) {
-    result.firstMatch = firstMatch.map(promoteForObject);
+    result.firstMatch = firstMatch.map(promoteAppiumOptionsForObject);
   } else if ('firstMatch' in originalCaps) {
     result.firstMatch = firstMatch;
   }
@@ -421,6 +429,7 @@ export {
   isStandardCap,
   stripAppiumPrefixes,
   promoteAppiumOptions,
+  promoteAppiumOptionsForObject,
   PREFIXED_APPIUM_OPTS_CAP,
 };
 
@@ -431,6 +440,11 @@ export {
  * @typedef {import('@appium/types').BaseDriverCapConstraints} BaseDriverCapConstraints
  */
 
+/**
+ * @template {Constraints} C
+ * @typedef {import('@appium/types').ConstraintsToCaps<C>} ConstraintsToCaps
+ */
+
 /**
  * @typedef ValidateCapsOpts
  * @property {boolean} [skipPresenceConstraint] - if true, skip the presence constraint

diff --git a/packages/base-driver/lib/index.js b/packages/base-driver/lib/index.js
@@ -35,6 +35,7 @@ export {
   isStandardCap,
   validateCaps,
   promoteAppiumOptions,
+  promoteAppiumOptionsForObject,
 } from './basedriver/capabilities';
 
 // Web socket helpers