refactor: update device handling in Automate tools #178

tech-sushant · 2025-11-08T22:11:18Z

No description provided.

…format and improve validation

Copilot

Pull Request Overview

This PR refactors device configuration from tuple-based arrays to object-based schemas across BrowserStack Automate and App Automate tools, improving type safety and API clarity.

Key Changes

Introduced MobileDeviceSchema and DeviceSchema with discriminated unions for platform-specific device configurations
Replaced tuple arrays with structured device objects in all schema definitions
Added conversion utilities (convertMobileDevicesToTuples) to bridge between new object format and existing tuple-based validators
Updated device descriptions to reflect object format usage

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/tools/sdk-utils/common/schema.ts	Defines new `MobileDeviceSchema` and `DeviceSchema` with discriminated unions; updates `RunTestsOnBrowserStackParamsShape` to use object-based device arrays
src/tools/sdk-utils/common/device-validator.ts	Adds `convertMobileDevicesToTuples` utility and normalizes "mac" to "macos" for consistent platform handling
src/tools/sdk-utils/bstack/sdkHandler.ts	Implements object-to-tuple conversion logic with platform-specific default values for browser and browserVersion fields
src/tools/appautomate.ts	Integrates conversion utilities and applies default mobile device configuration when none provided
src/tools/appautomate-utils/native-execution/constants.ts	Updates schema to use `MobileDeviceSchema` with object format
src/tools/appautomate-utils/appium-sdk/handler.ts	Applies mobile device conversion and default device handling
src/tools/appautomate-utils/appium-sdk/constants.ts	Updates schema to use `MobileDeviceSchema` with object format

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-09T15:09:16Z

src/tools/sdk-utils/common/schema.ts

+  }),
+  z.object({
+    platform: z.literal("android"),
+    deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24'"),


[nitpick] The device name description in the Android schema for DeviceSchema differs from the MobileDeviceSchema description (line 55). For consistency, consider using the same description: "Device name, e.g. 'Samsung Galaxy S24', 'Google Pixel 8'" to match the MobileDeviceSchema, which provides more examples.

Suggested change

deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24'"),

deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24', 'Google Pixel 8'"),

Copilot · 2025-11-09T15:09:16Z

src/tools/sdk-utils/common/schema.ts

+  }),
+  z.object({
+    platform: z.literal("ios"),
+    deviceName: z.string().describe("Device name, e.g. 'iPhone 12 Pro'"),


[nitpick] The iOS device name example in DeviceSchema uses 'iPhone 12 Pro', while the MobileDeviceSchema uses 'iPhone 15', 'iPhone 14 Pro' (line 60). For consistency and to reflect more current devices, consider updating this to match the examples in MobileDeviceSchema.

Suggested change

deviceName: z.string().describe("Device name, e.g. 'iPhone 12 Pro'"),

deviceName: z.string().describe("Device name, e.g. 'iPhone 15', 'iPhone 14 Pro'"),

Copilot · 2025-11-09T15:09:16Z

src/tools/sdk-utils/common/device-validator.ts

+  const normalizedDevices = devices.map((env) => {
+    const platform = (env[0] || "").toLowerCase();
+    if (platform === "mac") {
+      return ["macos", ...env.slice(1)];
+    }
+    return env;
+  });


The "mac" to "macos" normalization is performed here for tuple-based validation. However, the object-based schema in DeviceSchema (line 85 of schema.ts) accepts both "mac" and "macos" via z.enum(["mac", "macos"]). This means the conversion from objects to tuples in sdkHandler.ts should also handle "mac" consistently. The current implementation does handle this (line 35), but this normalization logic creates redundancy. Consider documenting this behavior or refactoring to have a single source of truth for platform normalization.

Copilot · 2025-11-09T15:09:17Z

src/tools/sdk-utils/common/device-validator.ts

+    }
+    return env;
+  });
+


Trailing whitespace detected. Remove the trailing whitespace on this empty line for cleaner code formatting.

Suggested change

Copilot · 2025-11-09T15:09:17Z

src/tools/sdk-utils/common/schema.ts

    ),
 };

+


Extra blank line detected. Remove one of the consecutive empty lines for consistent code formatting.

Suggested change

Copilot · 2025-11-09T15:09:17Z

src/tools/sdk-utils/common/schema.ts

+    platform: z.literal("windows"),
+    osVersion: z.string().describe("Windows version, e.g. '10', '11'"),
+    browser: z.string().describe("Browser name, e.g. 'chrome', 'firefox', 'edge'"),
+    browserVersion: z.string().describe("Browser version, e.g. '132', 'latest', 'oldest'"),


The browserVersion field is required in the Windows and macOS schemas but is treated as optional in the conversion logic (line 33, 40). This inconsistency could cause runtime errors when devices are validated against the schema. Consider making browserVersion optional in the schema using .optional() to match the conversion behavior, or ensure it's always provided in the conversion.

Copilot · 2025-11-09T15:09:17Z

src/tools/sdk-utils/common/schema.ts

+    platform: z.enum(["mac", "macos"]),
+    osVersion: z.string().describe("macOS version name, e.g. 'Sequoia', 'Ventura'"),
+    browser: z.string().describe("Browser name, e.g. 'safari', 'chrome'"),
+    browserVersion: z.string().describe("Browser version, e.g. 'latest'"),


The browserVersion field is required in the macOS schema but is treated as optional in the conversion logic (line 40 in sdkHandler.ts uses device.browserVersion || "latest"). This inconsistency could cause runtime errors when devices are validated against the schema. Consider making browserVersion optional in the schema using .optional() to match the conversion behavior.

Suggested change

browserVersion: z.string().describe("Browser version, e.g. 'latest'"),

browserVersion: z.string().describe("Browser version, e.g. 'latest'").optional(),

Copilot · 2025-11-09T15:09:18Z

src/tools/sdk-utils/common/schema.ts

+    platform: z.literal("android"),
+    deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24'"),
+    osVersion: z.string().describe("Android version, e.g. '14', '16', 'latest'"),
+    browser: z.string().describe("Browser name, e.g. 'chrome', 'samsung browser'"),


The browser field is required in the Android schema, but the conversion logic in sdkHandler.ts (line 47) treats it as optional with a default value (device.browser || "chrome"). This inconsistency could cause runtime errors when devices without a browser field are validated. Consider making browser optional in the schema using .optional() to match the conversion behavior.

Suggested change

browser: z.string().describe("Browser name, e.g. 'chrome', 'samsung browser'"),

browser: z.string().describe("Browser name, e.g. 'chrome', 'samsung browser'").optional(),

Copilot · 2025-11-09T15:09:18Z

src/tools/sdk-utils/common/schema.ts

+    platform: z.literal("ios"),
+    deviceName: z.string().describe("Device name, e.g. 'iPhone 12 Pro'"),
+    osVersion: z.string().describe("iOS version, e.g. '17', 'latest'"),
+    browser: z.string().describe("Browser name, typically 'safari'"),


The browser field is required in the iOS schema, but the conversion logic in sdkHandler.ts (line 54) treats it as optional with a default value (device.browser || "safari"). This inconsistency could cause runtime errors when devices without a browser field are validated. Consider making browser optional in the schema using .optional() to match the conversion behavior.

Suggested change

browser: z.string().describe("Browser name, typically 'safari'"),

browser: z.string().describe("Browser name, typically 'safari'").optional(),

Copilot · 2025-11-09T15:09:18Z

src/tools/sdk-utils/bstack/sdkHandler.ts

+  const tupleTargets: Array<Array<string>> = devices.map((device) => {
+    if (device.platform === "windows") {
+      return [
+        "windows",
+        device.osVersion,
+        device.browser,
+        device.browserVersion || "latest",
+      ];
+    } else if (device.platform === "mac" || device.platform === "macos") {
+      return [
+        "macos",
+        device.osVersion,
+        device.browser,
+        device.browserVersion || "latest",
+      ];
+    } else if (device.platform === "android") {
+      return [
+        "android",
+        device.deviceName,
+        device.osVersion,
+        device.browser || "chrome",
+      ];
+    } else if (device.platform === "ios") {
+      return [
+        "ios",
+        device.deviceName,
+        device.osVersion,
+        device.browser || "safari",
+      ];
+    } else {
+      throw new Error(`Unsupported platform: ${device.platform}`);
+    }
+  });


The device conversion logic contains repetitive code for handling different platforms. The fallback values (browserVersion || "latest", browser || "chrome", browser || "safari") should be documented or centralized. Consider extracting these defaults into constants to ensure consistency across the codebase and make maintenance easier.

…ogic

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-09T16:22:40Z

src/tools/appautomate.ts

+          (args.devices || []).length === 0
+            ? [["android", "Samsung Galaxy S24", "latest"]]
+            : convertMobileDevicesToTuples(args.devices || []);


The condition (args.devices || []).length === 0 checks the array length after defaulting to empty array, then uses args.devices || [] again in the conversion. This pattern is redundant. Simplify to: const devices: Array<Array<string>> = args.devices.length === 0 ? [['android', 'Samsung Galaxy S24', 'latest']] : convertMobileDevicesToTuples(args.devices); since the schema has .default([]) which ensures args.devices is never undefined.

Suggested change

(args.devices || []).length === 0

? [["android", "Samsung Galaxy S24", "latest"]]

: convertMobileDevicesToTuples(args.devices || []);

args.devices.length === 0

? [["android", "Samsung Galaxy S24", "latest"]]

: convertMobileDevicesToTuples(args.devices);

Copilot · 2025-11-09T16:22:40Z

src/tools/appautomate-utils/appium-sdk/handler.ts

+  const inputDevices = input.devices || [];
+  const devices: Array<Array<string>> =
    inputDevices.length === 0
      ? [["android", "Samsung Galaxy S24", "latest"]]
-      : inputDevices;
+      : convertMobileDevicesToTuples(inputDevices);


The default device [['android', 'Samsung Galaxy S24', 'latest']] is duplicated in both appautomate.ts (line 387) and appium-sdk/handler.ts (line 54). Consider defining this as a constant (e.g., DEFAULT_MOBILE_DEVICE) in a shared location to ensure consistency and ease maintenance.

…g in App Automate tools

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

refactor: update device handling in App Automate tools to use object …

10ace7e

…format and improve validation

tech-sushant requested a review from Copilot November 9, 2025 15:04

Copilot AI reviewed Nov 9, 2025

View reviewed changes

refactor: standardize platform naming and improve device validation l…

ff9a638

…ogic

tech-sushant requested a review from Copilot November 9, 2025 16:20

Copilot AI reviewed Nov 9, 2025

View reviewed changes

refactor: introduce DEFAULT_MOBILE_DEVICE for improved device handlin…

da2cb9f

…g in App Automate tools

tech-sushant requested a review from Copilot November 9, 2025 16:37

Copilot AI reviewed Nov 9, 2025

View reviewed changes

linting ++

ada7046

	deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24'"),
	deviceName: z.string().describe("Device name, e.g. 'Samsung Galaxy S24', 'Google Pixel 8'"),

	browserVersion: z.string().describe("Browser version, e.g. 'latest'"),
	browserVersion: z.string().describe("Browser version, e.g. 'latest'").optional(),

	browser: z.string().describe("Browser name, e.g. 'chrome', 'samsung browser'"),
	browser: z.string().describe("Browser name, e.g. 'chrome', 'samsung browser'").optional(),

refactor: update device handling in Automate tools #178

Are you sure you want to change the base?

refactor: update device handling in Automate tools #178

Conversation

tech-sushant commented Nov 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant