Move iPad and iPhone UI tests back to SauceLabs#73141
Conversation
The helper had a single caller after the mobile UI task was removed. Substitute its arguments as literals and drop the method definition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hamms
left a comment
There was a problem hiding this comment.
LGTM! Two minor suggestions, neither of them blocking. Thanks for the thorough tidying up!
| browser_names = $browsers.map {|b| b['name']}.uniq.sort | ||
| return 'Mobile UI' if browser_names.length > 1 && $browsers.all? {|b| mobile_browser?(b)} | ||
| "#{browser_names.join(' + ')} UI" | ||
| "#{$browsers.map {|b| b['name']}.join(' + ')} #{test_type}" |
There was a problem hiding this comment.
Makes me wonder whether we still want the if eyes? special-casing, or if it'd be cleaner to simplify that, too (in a follow-up PR)
There was a problem hiding this comment.
As long as all eyes tests are running in the same test suite, I think its best to keep the status page at test_status_Eyes.html and be named just "Eyes", but if you have a recommended simplification which does not alter the UX I'm open to it!
| # As of June 2026, our concurrency limit for desktop browser sessions in | ||
| # Device Farm within our prod AWS account is 150. |
There was a problem hiding this comment.
This comment feels a little contextless right now, particularly because the number here and the number used for --parallel below don't match.
What do you think about doing something like,
device_farm_concurrency_limit = 150.freeze # As of June 2026
And then using it below to informatively compute the value we want to use for parallelization?
There was a problem hiding this comment.
I agree that the comment is contextless. I mostly need a place to park this information and no place seems ideal. I wanted to keep the readme streamlined, but it really probably does better belong in the readme and not here.
for more background, this device farm desktop concurrency limit is shared between
- Chrome +Firefox UI tests during DTT (where this comment currently lives)
- Eyes tests during DTT
The value of 50 here is not strictly X% of the limit, rather it is hand-picked to be high enough for this test suite not to be the limiting factor in the DTT, while also being low enough to not overwhelm the test server thus indirectly impeding other test suites.
I'll open up a separate PR to move this info to the readme.
This is the final step in the current effort to reduce costs on AWS Device Farm (see proposal).
This PR does the following:
screenshots
slack output
Testing story
Deployment notes
After merging to staging:
UI/Eyes tests fail after I DTT?section inDeveloper of the Day (DotD / On Call) Resourcesdoc to include new status page links