Conversation
There was a problem hiding this comment.
Pull request overview
Adds native BrowserStack App Automate support to patrol_cli via new patrol bs ... commands, and refactors existing build commands/backends to better support artifact discovery and reuse.
Changes:
- Introduces
patrol bs android,patrol bs ios, andpatrol bs outputscommands (build/upload/schedule + optional artifact download). - Refactors build command wiring (reuses
BuildAndroidCommand/BuildIOSCommandinstances) and adjusts iOS.xctestrundiscovery. - Updates CLI version and dependencies (adds
archive).
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/patrol_cli/lib/src/runner/patrol_command_runner.dart | Wires new BrowserStack command group and reuses build subcommands. |
| packages/patrol_cli/lib/src/commands/browserstack/* | Adds BrowserStack API client + commands for Android/iOS uploads and outputs/artifact downloads. |
| packages/patrol_cli/lib/src/commands/build_android.dart | Improves APK path discovery and logging for build outputs. |
| packages/patrol_cli/lib/src/commands/build_ios.dart | Adds flavor-aware output paths; updates .xctestrun path logging. |
| packages/patrol_cli/lib/src/ios/ios_test_backend.dart | Changes .xctestrun discovery implementation and return type. |
| packages/patrol_cli/lib/src/runner/patrol_command.dart | Adds arg/global-results override hooks for command reuse. |
| packages/patrol_cli/test/** | Updates/extends tests for iOS backend and iOS build output paths. |
| packages/patrol_cli/pubspec.yaml | Bumps version and adds archive dependency. |
| packages/patrol_cli/lib/src/base/constants.dart | Keeps CLI version constant in sync. |
| packages/patrol_devtools_extension/pubspec.lock | Updates locked transitive dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| _logger.detail( | ||
| 'Found ${files.length} match(es), the first one will be used', | ||
| throw FileSystemException( |
There was a problem hiding this comment.
xcTestRunPath() now throws a raw FileSystemException when no match is found. Call sites like IOSTestBackend.execute() don't catch this, so users may get an unhandled exception instead of a clean throwToolExit(...) message. Consider converting the failure back to a ToolExit (or catching FileSystemException at call sites and rethrowing with throwToolExit).
| throw FileSystemException( | |
| throwToolExit( |
| for (final root in [Directory('build'), Directory('android')]) { | ||
| if (!root.existsSync()) { | ||
| continue; | ||
| } | ||
| try { | ||
| for (final entity in root.listSync(recursive: true)) { | ||
| if (entity is File && | ||
| entity.path.contains('outputs/apk/') && | ||
| entity.path.endsWith(apkName)) { | ||
| return entity.path; |
There was a problem hiding this comment.
_findApkPath() searches for alternative APK locations by checking entity.path.contains('outputs/apk/'). This uses a hard-coded forward slash and will fail on Windows paths (e.g. outputs\apk\...), preventing fallback discovery. Consider normalizing paths or using package:path utilities to perform this check in a platform-independent way.
| // Get session ID | ||
| final devices = currentBuildData['devices'] as List<dynamic>?; | ||
| if (devices == null || devices.isEmpty) { | ||
| _logger.err('No devices found in build data'); | ||
| return BsExitCodes.buildError; | ||
| } | ||
|
|
||
| final sessions = | ||
| (devices.first as Map<String, dynamic>)['sessions'] as List<dynamic>?; | ||
| if (sessions == null || sessions.isEmpty) { | ||
| _logger.err('No sessions found in build data'); | ||
| return BsExitCodes.buildError; | ||
| } | ||
|
|
||
| final sessionId = | ||
| (sessions.first as Map<String, dynamic>)['id'] as String?; | ||
| if (sessionId == null) { | ||
| _logger.err('Could not find session ID in build data'); | ||
| return BsExitCodes.buildError; | ||
| } |
There was a problem hiding this comment.
BsOutputsCommand picks only devices.first and sessions.first, so for multi-device runs it will download artifacts for just one session while silently ignoring the rest. This contradicts --devices allowing multiple devices. Consider iterating over all devices/sessions and naming outputs per device/session (e.g., include device name + session id) so users get complete artifacts.
| final multipartFile = http.MultipartFile( | ||
| 'file', | ||
| fileStream, | ||
| fileLength, | ||
| filename: file.path.split('/').last, | ||
| ); |
There was a problem hiding this comment.
filename: file.path.split('/').last is not platform-independent (Windows uses \). Use package:path's basename(file.path) to ensure correct filenames across OSes.
| Future<void> _createZip( | ||
| String sourceDir, | ||
| String outputPath, | ||
| String rootDirName, | ||
| ) async { |
There was a problem hiding this comment.
_createZip() takes rootDirName but never uses it, which is confusing (and may trigger lint for unused parameters). Either remove the parameter or use it to control how paths are stored inside the archive.
| 'Environment configuration from a provided path that will be available ' | ||
| 'to the app under test.', |
There was a problem hiding this comment.
There is a trailing space in this help text (after available). This can lead to inconsistent formatting in --help output; remove the extra whitespace.
| 'Environment configuration from a provided path that will be available ' | |
| 'to the app under test.', | |
| 'Environment configuration from a provided path that will be available' | |
| ' to the app under test.', |
| test('prints correct paths for simulator build with flavor', () { | ||
| command.printBinaryPaths( | ||
| simulator: true, | ||
| buildMode: 'Debug', | ||
| flavor: 'dev', | ||
| ); | ||
|
|
||
| verify( | ||
| () => mockLogger.info( | ||
| '${join('build', 'ios_integ', 'Build', 'Products', 'Debug-dev-iphonesimulator', 'Runner.app')} (app under test)', | ||
| ), | ||
| ).called(1); | ||
| verify( | ||
| () => mockLogger.info( | ||
| '${join('build', 'ios_integ', 'Build', 'Products', 'Debug-dev-iphonesimulator', 'RunnerUITests-Runner.app')} (test instrumentation app)', | ||
| ), | ||
| ).called(1); |
There was a problem hiding this comment.
BuildIOSCommand.printBinaryPaths() now logs App: ... / Test App: ... (see build_ios.dart), but these new tests still assert the old output format with (<app under test>) suffixes. Align the expectations with the new log output (or keep the previous log format if it’s part of the CLI contract).
| ); | ||
|
|
||
| _logger.info('$xcTestRunPath (xctestrun file)'); | ||
| _logger.info('Test Plan: $xcTestRunPath'); |
There was a problem hiding this comment.
The log label Test Plan: is misleading here because the value is the .xctestrun path. Consider logging something like xctestrun file: (and keep naming consistent with other platforms, e.g. macOS currently logs (... xctestrun file)).
| _logger.info('Test Plan: $xcTestRunPath'); | |
| _logger.info('xctestrun file: $xcTestRunPath'); |
| for (final entity in directory.listSync()) { | ||
| final name = entity.basename; | ||
| if (name.startsWith(prefix) && | ||
| name.contains(platformSdk) && | ||
| name.endsWith('.xctestrun')) { |
There was a problem hiding this comment.
entity.basename is not a member of FileSystemEntity and there is no local extension providing it (searching the repo shows no basename extension). This will not compile; use basename(entity.path) from package:path/path.dart (or p.basename).
Summary
This PR adds native BrowserStack App Automate integration to the Patrol CLI, enabling users to build, upload, and run Patrol tests on BrowserStack's directly from the CLI.
New Commands
patrol bs android- Build and upload Android APKs to BrowserStack for Espresso testingpatrol bs ios- Build and upload iOS apps to BrowserStack for XCUITest testingpatrol bs outputs <build_id>- Get outputs and artifacts from a completed BrowserStack buildFeatures
--api-paramsfor advanced BrowserStack configurationConfiguration
Credentials and devices can be set via CLI options or environment variables:
--credentialsPATROL_BS_CREDENTIALS(format:username:access_key)--devicesPATROL_BS_ANDROID_DEVICES/PATROL_BS_IOS_DEVICES--api-paramsPATROL_BS_ANDROID_API_PARAMS/PATROL_BS_IOS_API_PARAMS--projectPATROL_BS_PROJECTUsage Example
Run Android tests on BrowserStack
patrol bs android
--target patrol_test/app_test.dart
--credentials "user:key"
--devices '["Google Pixel 8-14.0"]'
--download-outputs
Run iOS tests on BrowserStack
patrol bs ios
--target patrol_test/app_test.dart
--credentials "user:key"
--devices '["iPhone 15-17"]'
-d
The
--download-outputs(-d) flag makes the command wait for the test run to complete, download all artifacts (logs, videos, reports), and exit with an appropriate code based on test results.Get outputs from existing build
patrol bs outputs abc123def --output-dir ./results