chore: Retry Cinema 4D launch if it fails first time on tests. - #522
chore: Retry Cinema 4D launch if it fails first time on tests.#522karthikbekalp wants to merge 3 commits into
Conversation
Pull request was converted to draft
Signed-off-by: Karthik Bekal Pattathana <133984042+karthikbekalp@users.noreply.github.com>
Signed-off-by: Karthik Bekal Pattathana <133984042+karthikbekalp@users.noreply.github.com>
Signed-off-by: Karthik Bekal Pattathana <133984042+karthikbekalp@users.noreply.github.com>
7f50b7d to
ccfc308
Compare
| if ($ssmSessionId) { Write-Host "SSM session id: $ssmSessionId" } else { Write-Host "SSM session id: <not captured>" } | ||
|
|
||
| if (-not $ready) { | ||
| Write-Error "SSM port forward failed after 2 attempts ($lastStatus)" |
There was a problem hiding this comment.
The captured SSM logs are never printed, so the failure output is less informative than before this change.
$ssmOutLog / $ssmErrLog are created per attempt (lines 124-130) but nothing ever reads them: the failure path here only prints $lastStatus ("exit code N" / "process still running"), and the old code it replaced did dump the log (if (Test-Path $ssmLog) { Get-Content $ssmLog }). Since capturing the AWS CLI stderr is one of the stated goals of this change, the retry loop / final failure should echo both logs, e.g. before exit 1:
foreach ($a in 1..2) {
foreach ($f in @("ssm-$a.out.log", "ssm-$a.err.log")) {
$p = Join-Path $env:RUNNER_TEMP $f
if (Test-Path $p) { Write-Host "--- $f ---"; Get-Content $p }
}
}Worth dumping the failing attempt inside the loop too (next to the Write-Warning on line 165), so a run that fails on attempt 1 and succeeds on attempt 2 still leaves the diagnostic behind.
|
|
||
| [envs.integ.scripts] | ||
| test = "pytest --no-cov {args:test/integ} -vvv --numprocesses=1" | ||
| test = "pytest --no-cov {args:test/integ} -vvv --numprocesses=0 -s -o faulthandler_timeout=600 -o faulthandler_exit_on_timeout=true" |
There was a problem hiding this comment.
faulthandler_timeout is per test item (armed/cancelled around each item, including its fixture setup and teardown), and faulthandler_exit_on_timeout=true makes it call os._exit from the watchdog thread. Two consequences worth checking before merging:
1. 600s may be tight for a single case, and the retry added in this PR can blow past it. The declared timeouts inside one export attempt already sum to a large fraction of the budget: _C4D_BOOT_TIMEOUT_S + _DIALOG_VISIBLE_TIMEOUT_S = 240s for the UIA app, then _wait_for_submitter_dialog (60s), _wait_for_queue_environment_loading (60s), and the two _press_export_bundle waits (60s + 60s) — ~480s before the scene build and the assert_openjd_run_with_cinema4d_successful render are counted. With _export_job_bundle_via_submitter now looping twice, a case that hits the startup crash on attempt 1 and then renders normally on attempt 2 can plausibly exceed 600s. That converts a recovered run into a hard abort — the opposite of what the retry is for.
2. When it does fire, the whole session dies with no report. os._exit skips pytests teardown and report flush, so: the timing-out test produces no failure entry, every remaining test in the session is silently never run (the CI job just fails), and _export_job_bundle_via_submitters finally never executes — leaving the Cinema 4D process alive and the c4d-submitter-ui-* staging dir on disk. Harmless on ephemeral windows-latest, but it does mean the log ends abruptly with only a thread dump and no indication of how many cases were skipped.
Suggest sizing the timeout off the actual worst-case case duration (with retry) plus margin, and noting in test/AGENTS.md that the trigger aborts the entire run rather than failing one test — the current doc wording ("pytest dumps all Python thread stacks and exits") reads as if only that test is affected.
This requires more investigation and run multiple times to figure out the issue. Parking this as draft while I work on higher priorities.
What was the problem/requirement? (What/Why)
The Windows integration workflow exposed two startup failure modes:
0xC0000005(STATUS_ACCESS_VIOLATION) before its submitter accessibilityapplication appeared. The test continued waiting for UI Automation, which
obscured the native process failure.
did not retain the AWS CLI process or capture its stderr, so it could only
report a port timeout.
Test-NetConnectionalso made the nominal timeoutsubstantially longer than the message indicated.
Failed jobs:
https://github.com/aws-deadline/deadline-cloud-for-cinema-4d/actions/runs/31621623631/job/94197613785
https://github.com/aws-deadline/deadline-cloud-for-cinema-4d/actions/runs/31669886824/job/94352428414
What was the solution? (How)
The Windows integration test now monitors the launched Cinema 4D process before
each UI Automation scan:
codes and restarts Cinema 4D once.
succeeds.
code.
Only an early process exit is retried. Accessibility timeouts and failures after
startup are not retried.
The integration tests now run in-process with uncaptured output so Cinema 4D
and xa11y progress is visible immediately. If a test runs for ten minutes,
pytest's faulthandler dumps every Python thread and exits rather than waiting
for the GitHub Actions job timeout.
The Windows SSM setup now:
aws ssm start-sessionprocess and reports an early exit code.five-second intervals.
out.
What is the impact of this change?
This change only affects the integration test harness and Windows integration
workflow. It makes the suite resilient to a rare, transient Cinema 4D startup
crash and provides actionable AWS CLI and Session Manager diagnostics if the
license tunnel fails again.
There is no change to the Cinema 4D submitter, adaptor, customer workflows, or
production behavior.
How was this change tested?
Unit tests:
358 passed, 6 skippedhatch run lint: passedWorkflow YAML parsing: passed
git diff --check: passedHave you run the unit tests?
Yes.
hatch run testcompleted with358 passed, 6 skipped.Have you run the integration tests? (Add your integration test report below)
Not locally. The full integration suite requires a Windows environment with
Cinema 4D installed, GitHub OIDC credentials, and access to the license
infrastructure. End-to-end validation must run in the versioned Windows
integration workflow.
Have you made changes to the submitter?
No. The changes are limited to the integration test harness, its Hatch
command, the Windows workflow, and test documentation.
Was this change documented?
The modified integration-test functions include updated docstrings, and
test/AGENTS.mddocuments the in-process execution and hang diagnostics. NoREADME, schema, or customer-facing documentation changes are required.
Is this a breaking change?
No. This change is limited to integration test behavior and does not modify any
public contract or customer-facing functionality.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.