Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .github/workflows/device-tests-ios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,7 @@ jobs:
run: pwsh ./scripts/device-test.ps1 ios -Build

- name: Run Tests
id: first-test-run
continue-on-error: true
timeout-minutes: 40
run: pwsh scripts/device-test.ps1 ios -Run -Verbose

- name: Retry Tests (if previous failed to run)
if: steps.first-test-run.outcome == 'failure'
timeout-minutes: 40
timeout-minutes: 60
run: pwsh scripts/device-test.ps1 ios -Run -Verbose

- name: Run Integration Tests
Expand Down
41 changes: 40 additions & 1 deletion scripts/device-test-utils.ps1
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
function Reset-IosSimulator {
[CmdletBinding()]
param(
[Parameter(Mandatory)]
[PSCustomObject]$SimInfo
)

$udid = $SimInfo.Udid

Write-Host "Hard-resetting simulator $($SimInfo.Name) [$udid]..."

# Shut down & delete the old device
xcrun simctl shutdown $udid 2>&1 | Out-Null
xcrun simctl delete $udid 2>&1 | Out-Null

# Restart the CoreSimulator service to clear any daemon-level state left
# over from a simulator crash (this is what makes a fresh runner "work").
Write-Host "Restarting CoreSimulator service..."
sudo launchctl kickstart -kp system/com.apple.CoreSimulator.CoreSimulatorService 2>&1 | Out-Null
Start-Sleep -Seconds 5 # give the daemon time to re-initialise

# Create a brand-new simulator with the same device type & runtime
$newUdid = (& xcrun simctl create $SimInfo.Name $SimInfo.DeviceType $SimInfo.Runtime).Trim()

Check warning on line 23 in scripts/device-test-utils.ps1

View check run for this annotation

@sentry/warden / warden: find-bugs

Missing error check on xcrun simctl create can cause cascading failures with invalid UDID

Line 23 captures the output of `xcrun simctl create` but does not check `$LASTEXITCODE`. If the create command fails (e.g., invalid DeviceType/Runtime, or resource exhaustion), the error message text becomes the `$newUdid` value. Subsequent operations at lines 27-28 will attempt to boot this garbage string as a UDID, and line 32 returns it to the caller, causing hard-to-diagnose CI failures.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error check on xcrun simctl create can cause cascading failures with invalid UDID

Line 23 captures the output of xcrun simctl create but does not check $LASTEXITCODE. If the create command fails (e.g., invalid DeviceType/Runtime, or resource exhaustion), the error message text becomes the $newUdid value. Subsequent operations at lines 27-28 will attempt to boot this garbage string as a UDID, and line 32 returns it to the caller, causing hard-to-diagnose CI failures.

Verification

Read Reset-IosSimulator function completely. Traced flow from xcrun simctl create at line 23 through simctl boot at line 27 and return at line 32. Confirmed no $LASTEXITCODE check or try/catch around the create operation. Compared with other functions in the file which also lack such checks.

Identified by Warden find-bugs · 4YX-2ML

Write-Host "Created new simulator $($SimInfo.Name) [$newUdid]"

# Boot and wait
xcrun simctl boot $newUdid 2>&1 | Out-Null
xcrun simctl bootstatus $newUdid -b

# Return updated info
$SimInfo.Udid = $newUdid
return $newUdid
}

function Install-XHarness
{
if (!(Get-Command xharness -ErrorAction SilentlyContinue))
Expand Down Expand Up @@ -120,5 +154,10 @@
}

Write-Verbose ("Selected simulator: {0} ({1}) [{2}]" -f $selected.Device.name, $selected.Device.deviceTypeIdentifier, $selected.Device.udid)
return $selected.Device.udid
return [PSCustomObject]@{
Udid = $selected.Device.udid
Name = $selected.Device.name
DeviceType = $selected.Device.deviceTypeIdentifier
Runtime = $runtimeKey
}

Check failure on line 162 in scripts/device-test-utils.ps1

View check run for this annotation

@sentry/warden / warden: code-review

Breaking change: Get-IosSimulatorUdid return type change breaks ios.Tests.ps1

The function `Get-IosSimulatorUdid` now returns a PSCustomObject instead of a string UDID. However, `integration-test/ios.Tests.ps1` calls this function at line 11 and uses `$simulator` directly with `xcrun simctl install`, `xcrun simctl uninstall`, `xcrun simctl spawn`, and `xcrun simctl launch` commands which expect a string UDID. This will cause runtime failures in the iOS integration tests because PowerShell will coerce the object to a string representation instead of the actual UDID value.
Comment on lines +157 to +162
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking change: Get-IosSimulatorUdid return type change breaks ios.Tests.ps1

The function Get-IosSimulatorUdid now returns a PSCustomObject instead of a string UDID. However, integration-test/ios.Tests.ps1 calls this function at line 11 and uses $simulator directly with xcrun simctl install, xcrun simctl uninstall, xcrun simctl spawn, and xcrun simctl launch commands which expect a string UDID. This will cause runtime failures in the iOS integration tests because PowerShell will coerce the object to a string representation instead of the actual UDID value.

Verification

Read scripts/device-test-utils.ps1 to see the return type change from string to PSCustomObject. Read integration-test/ios.Tests.ps1 which calls Get-IosSimulatorUdid at line 11 and uses $simulator directly at lines 43, 53-54, 68-69, 70-74 with xcrun simctl commands. Confirmed ios.Tests.ps1 is not in the list of files modified in this PR, meaning it won't be updated to handle the new return type.

Identified by Warden code-review · 5YQ-FP3

}
52 changes: 51 additions & 1 deletion scripts/device-test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ try
$Tfm = 'net10.0'
}
$arch = (!$IsWindows -and $(uname -m) -eq 'arm64') ? 'arm64' : 'x64'
$udid = $null
if ($Platform -eq 'android')
{
$Tfm += '-android'
Expand Down Expand Up @@ -64,8 +65,11 @@ try

$udid = Get-IosSimulatorUdid -Verbose
if ($udid) {
$simInfo = $udid # Get-IosSimulatorUdid now returns an object
$udid = $simInfo.Udid
$arguments += @('--device', $udid)
} else {
$simInfo = $null
Write-Host "No suitable simulator found; proceeding without a specific --device"
}
}
Expand All @@ -84,13 +88,59 @@ try
{
Install-XHarness
Remove-Item -Recurse -Force test_output -ErrorAction SilentlyContinue

if ($Platform -eq 'ios' -and $udid)
{
Write-Host "Ensuring simulator is booted and ready..."
if ($CI)
{
# On CI, do a full reset to guarantee a clean simulator
$udid = Reset-IosSimulator -SimInfo $simInfo
# Update arguments with new UDID
for ($i = 0; $i -lt $arguments.Count; $i++) {
if ($arguments[$i] -eq '--device' -and ($i + 1) -lt $arguments.Count) {
$arguments[$i + 1] = $udid
break
}
}
}
else
{
xcrun simctl boot $udid 2>&1 | Out-Null # no-op if already booted
xcrun simctl bootstatus $udid -b # block until fully operational
}
}

try
{
if ($VerbosePreference)
{
$arguments += '-v'
}
xharness $group test $arguments --output-directory=test_output

$maxAttempts = 3
for ($attempt = 1; $attempt -le $maxAttempts; $attempt++)
{
if ($attempt -gt 1)
{
Write-Host "Retrying xharness (attempt $attempt of $maxAttempts)..."
Remove-Item -Recurse -Force test_output -ErrorAction SilentlyContinue
if ($Platform -eq 'ios' -and $simInfo)
{
$udid = Reset-IosSimulator -SimInfo $simInfo
# Update arguments with new UDID
for ($i = 0; $i -lt $arguments.Count; $i++) {
if ($arguments[$i] -eq '--device' -and ($i + 1) -lt $arguments.Count) {
$arguments[$i + 1] = $udid
break
}
}
}
}
xharness $group test $arguments --output-directory=test_output
if ($LASTEXITCODE -eq 0) { break }
}

if ($LASTEXITCODE -ne 0)
{
$testResultsXml = './test_output/TestResults.xml'
Expand Down
Loading