tests: arm Crashpad in sgl_tests so teardown faults produce a minidump (#1062)

[nv-slang-bot]

Summary

• Status: Diagnostics-only instrumentation for sgl_tests exits nonzero after all tests pass (teardown/exit-code flake reddening cross-repo SlangPy Tests check) #1062. Built locally (linux-gcc, -DSGL_ENABLE_CRASHPAD=ON); offline coworker review: APPROVE. Ready for maintainer review/merge consideration.
• Link: References sgl_tests exits nonzero after all tests pass (teardown/exit-code flake reddening cross-repo SlangPy Tests check) #1062. Supersedes tests: exit sgl_tests with doctest status, bypassing post-main teardown (#1062) #1064 (rejected _Exit approach).
• Verdict: Arms Crashpad in the C++ sgl_tests harness so the next teardown fault should produce a stackwalkable minidump (when the handler starts successfully). Does not alter the exit-code path — real test failures and mid-test crashes still red the job.
• Next-action: Maintainer review / merge consideration. On the next occurrence of the flake, download the crash-reports-<os>-<platform>-<compiler>-<config> artifact and stackwalk the teardown fault frame.
• Blocker: None. Cannot be validated by reproducing the (non-deterministic; observed only on Windows so far) flake locally; this only adds capture infrastructure.

Motivation

sgl_tests (the C++ doctest harness, run via python tools/ci.py unit-test-cpp) intermittently exits nonzero after a green doctest summary — the fault happens during teardown, after context.run() has already returned success. tools/ci.py's run_command then raises RuntimeError on the nonzero exit, reddening the SlangPy Tests check (including on unrelated slang PRs via repository_dispatch). Two confirmed occurrences with an identical signature (runs 27965567210, 29232873855).

Crashpad is compiled into the CI binary (ci.py sets -DSGL_ENABLE_CRASHPAD=ON for the crashpad matrix rows) and an upload step is configured, but the handler was only armed on the Python side (slangpy/testing/plugin.py → crashpad.start_handler()). The C++ sgl_tests main() never called start_handler(), so a teardown fault wrote no minidump — the upload step produced no usable artifact and the log showed only a bare exit 1 with no stack. We are blind to the exact teardown frame.

Note that the cross-repo "SlangPy Tests" check that reds slang PRs runs via repository_dispatch through .github/workflows/ci-latest-slang.yml → the .github/actions/build-and-test-with-slang composite action, which has its own crashpad upload step; the main ci.yml uploader services the in-repo runs. This PR updates the composite action (the #1062 lane); the ci.yml counterpart is a maintainer follow-up (the bot lacks the workflows App permission to push it).

Maintainer jhelferty-nv greenlit arming Crashpad for the C++ harness (issue #1062 comment 5320479090), after skallweitNV rejected the earlier _Exit-based exit-code masking on #1064.

Proposed solution

Arm Crashpad in sgl_tests main() immediately after sgl::testing::static_init() and before context.run(), guarded on SGL_HAS_CRASHPAD. This reuses the already-compiled-in Crashpad support, so the primary gap was the missing runtime start_handler() call on the C++ side. A secondary gap in the upload path also had to close for the dump to actually reach an artifact: the CI uploaders archived only .crashpad/reports/ (missing the POSIX pending/ dump) and upload-artifact skips hidden files, so the dot-directory's contents were excluded. This PR closes both on the repository_dispatch "SlangPy Tests" lane (the #1062 target); the identical ci.yml edit for in-repo runs is a maintainer follow-up (the bot lacks the workflows App permission). With the handler armed and the upload fixed, the next teardown fault should leave a stackwalkable minidump — when the handler starts successfully.

This is instrumentation, not a fix: the exit-code path is untouched, so a real test failure (result != 0) and a mid-test crash both still red the job exactly as today. It captures signal for the next occurrence; it does not fix the underlying teardown fault.

Change summary

File	Change
tests/sgl/sgl_tests.cpp	Add explicit #include "sgl/core/config.h" (defines the guard macro) and #include "sgl/utils/crashpad.h" (the API). Arm sgl::crashpad::start_handler({}, ".crashpad") under #if SGL_HAS_CRASHPAD, after static_init() and before context.run(), wrapped in a try/catch that logs and continues.
.github/actions/build-and-test-with-slang/action.yml	Broaden the "Upload Crashpad Reports" artifact glob from .crashpad/reports/ to the whole .crashpad/ database so a POSIX teardown minidump (which lands in pending/) is archived even when the crash is in the C++ step, and set include-hidden-files: true so upload-artifact does not skip the dot-directory's contents. This composite action services the repository_dispatch "SlangPy Tests" lane — the one that reds slang PRs, i.e. the actual #1062 target.

Maintainer follow-up (not in this PR): .github/workflows/ci.yml's "Upload Crashpad Reports" step needs the identical two-line edit (path: .crashpad/ + include-hidden-files: true) for the in-repo push/PR runs. The bot's GitHub App token lacks the workflows permission, so it cannot push a workflow-file edit; a maintainer should apply that two-line edit. The cross-repo lane above (the #1062 target) is fully covered by this PR.

Concepts and vocabulary

• SGL_HAS_CRASHPAD — the source-visible compile define, generated into sgl/core/config.h (src/sgl/CMakeLists.txt), set ON only when the cmake option SGL_ENABLE_CRASHPAD=ON and the crashpad package is found. This is the correct guard; SGL_ENABLE_CRASHPAD is only the cmake option name and is not visible to source. It is the same guard src/sgl/utils/crashpad.cpp uses.
• Crashpad database vs. reports dir — start_handler(handler, database, ...) writes minidumps under <database>. On Windows the .dmp files land directly in <database>/reports/; on POSIX they go to <database>/pending/ and the Python harness copies them into reports/.

Process report

Include footgun (why the two explicit includes). #if SGL_HAS_CRASHPAD silently evaluates to #if 0 if config.h is not in the translation unit — the code would then compile to nothing and the instrumentation would silently do nothing. Today sgl_tests.cpp reaches config.h only transitively (e.g. via sgl/device/agility_sdk.h and sgl/device/device.h). Rather than depend on those incidental includes surviving a future refactor, the guard's defining header is included explicitly, alongside the crashpad API header. The sgl target exposes the generated-config include directory as PUBLIC (src/sgl/CMakeLists.txt) and sgl_tests links sgl, so both includes resolve.

Explicit database path (why not a bare start_handler()). start_handler() defaults its database to runtime_directory()/crashpad_database — next to the test binary. But CI archives the .crashpad directory relative to the repo root, and the Python harness (slangpy/testing/crashpad.py) already passes database=<repo_root>/.crashpad. A bare start_handler() in C++ would write the minidump to a directory the upload step never looks at, so the whole point of the change — getting the dump archived — would be lost. Passing the explicit ".crashpad" path makes the C++ harness land minidumps in the same archived location the Python side uses. (The path is relative to the process CWD; ci.py invokes sgl_tests from the repo root, matching how the upload glob and the Python PROJECT_DIR/.crashpad resolve.)

Best-effort arming. start_handler() throws (std::runtime_error via SGL_THROW) if the handler executable is missing or the handler fails to start. Since this is diagnostics that must never perturb the test result, the call is wrapped in a try/catch that logs a warning and continues — mirroring the Python harness (slangpy/testing/crashpad.py), which catches and prints on failure.

Archiving the POSIX dump (why the CI upload change). Both CI uploaders — ci.yml for in-repo runs, and the build-and-test-with-slang composite action for the repository_dispatch lane — archived only .crashpad/reports/. On Windows (where both observed occurrences happened) Crashpad writes the .dmp directly into <database>/reports/, matching that old glob. On POSIX it writes to <database>/pending/, and only the Python harness's terminal-summary hook copies pending/ → reports/. But when the C++ unit-test-cpp step is the one that crashes, its nonzero exit fails that step and the success()-gated unit-test-python step is skipped, so the copy never runs. This PR updates the composite action (the #1062 target lane) to archive the whole .crashpad/ database (the upload step is if: always()), which captures the pending/ dump regardless. upload-artifact@v7 excludes hidden files by default and treats everything under a dot-directory as hidden, so include-hidden-files: true is required or the artifact would be silently empty (masked by if-no-files-found: ignore). The equivalent ci.yml edit is a maintainer follow-up (see the note in the change summary): the bot cannot push workflow files.

Invariant preserved. The teardown sequence and return result are unchanged; nothing about the exit code is altered. A genuine failing test still returns nonzero and reds the job; a mid-test crash always reds the job and is captured by Crashpad when the handler started successfully. The change only means that when the teardown fault next fires, a minidump should be available to stackwalk (again, when the handler started successfully).

Interpreting a null result. Crashpad captures a hardware fault or abort() (SIGSEGV/SIGABRT-class), not a graceful nonzero return. If #1062's teardown failure turns out to be a clean nonzero exit rather than a crash signal, no minidump is produced — so a "no dump uploaded" on a future red run does not necessarily mean arming failed; a null result stays inconclusive (it could be a graceful nonzero exit, a handler that failed to start, or an upload miss). A dump, on the other hand, is a positive signal that pins the fault. This is still the correct first diagnostic step: capturing a dump collapses the solution space, while its absence remains inconclusive.

PR #1064 (the rejected _Exit approach) is superseded by this instrumentation-only approach and will be closed in its favor.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

nv-slang-bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[jhelferty-nv]

Automated notice (PR board sync) — do not reply to this comment.

Auto-assigned @jkiviluoto-nv as shepherd for this Bot PR.

FYI for maintainers: committer signal on the changed files is higher for skallweitNV than for the auto-requested reviewer (jkiviluoto-nv). They were not auto-requested; a human may optionally add them as a reviewer.

[skallweitNV]

I'm not sure about setting the artifact path to ./crashpad. Won't we miss the reports that are currently reported from the pytest run?

[nv-slang-bot]

Good question — the short answer is no, we don't lose the pytest reports; .crashpad/ is a strict superset of .crashpad/reports/.

Concretely:

• The Python/pytest harness writes its database to <repo>/.crashpad (slangpy/testing/crashpad.py), and on Windows dumps land directly in .crashpad/reports/, while on POSIX its pytest_terminal_summary hook copies pending/ → reports/. Both live under .crashpad/, so uploading .crashpad/ captures everything the old .crashpad/reports/ path did, plus pending/.
• The artifact name: is unchanged (crash-reports-…); all the report files that were uploaded before are still uploaded, now alongside pending/ (widening the upload root can change their paths inside the archive, but nothing is dropped).

The reason for widening the path is the case this PR targets: when the C++ sgl_tests step is the one that faults during teardown (#1062), its dump lands in .crashpad/pending/ on POSIX and the pytest copy-step never runs (that step is skipped once the C++ step has failed), so a reports/-only upload would miss it. Uploading the whole DB covers both the pytest reports and the C++ pending dumps.

On include-hidden-files: true: .crashpad is a dot-directory, and upload-artifact@v7 excludes hidden files by default — its docs define hidden as "any file beginning with . or files within folders beginning with .". So without that flag the dot-directory's contents are skipped, and if-no-files-found: ignore would silently mask the empty result. (By the same rule, the previous .crashpad/reports/ path was itself under the .crashpad dot-directory without the flag, so it was at risk of being skipped too.)

If you'd rather keep the upload narrower, I can restrict it to path: | / .crashpad/reports/ / .crashpad/pending/ (still with include-hidden-files: true) instead of the whole DB — happy to change it either way. The whole-DB upload is just simpler and gives a bit more postmortem context (the <pid>.txt test-name breadcrumbs and any post-processed .txt/.json).

Note this only touches the build-and-test-with-slang composite action, which both jobs in ci-latest-slang.yml use — the scheduled/manual build job and the repository_dispatch build-pr job that posts the "SlangPy Tests" status (the lane that reds slang PRs). The same two-line change is still needed in .github/workflows/ci.yml for the regular in-repo push/PR lanes; I couldn't include it here because the bot's push of that file was rejected ("refusing to allow a GitHub App to create or update workflow .github/workflows/ci.yml without workflows permission"), so it's flagged as a maintainer follow-up in the description.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}