Skip to content

No Popups from test crashes#745

Open
johnml1135 wants to merge 4 commits intomainfrom
no-popups-from-test-crashes
Open

No Popups from test crashes#745
johnml1135 wants to merge 4 commits intomainfrom
no-popups-from-test-crashes

Conversation

@johnml1135
Copy link
Contributor

@johnml1135 johnml1135 commented Mar 6, 2026

Summary

FieldWorks tests were still inheriting interactive desktop assertion behavior from both native and managed code paths, so some failures could block the run with modal popups instead of surfacing as ordinary test failures. Those popups exist for a legitimate reason in normal developer debugging: they force attention, allow debugger attachment, and prevent silent continuation after a serious assertion. This change keeps that interactive behavior for normal app runs, but makes test runs explicitly non-interactive, fail-fast, and diagnosable across the managed runner, native runner, shared test bootstrap, and projects that opt out of the shared bootstrap.

What Was Happening

This PR addresses a class of test failures where the process did not simply fail the test and continue. Instead, some failures flowed through existing desktop-era assertion and exception infrastructure that was designed for an interactive developer sitting at the machine.

In practice there were two main problem paths:

  • native C++ assertions could route through DebugProcs.dll and show a Windows Abort/Retry/Ignore message box
  • managed assertions and last-chance exceptions could surface through trace listeners or WinForms exception hooks rather than as a clean, visible test failure

That behavior is acceptable for interactive local debugging, but it is the wrong default for unattended test runs and CI because a popup can block the entire run indefinitely.

Why Popups Exist At All

The popup behavior is not accidental. FieldWorks is a long-lived Windows desktop application with native and managed components, and modal assertion UI is useful during manual debugging because it:

  • immediately stops the developer on a serious failure
  • offers a debugger-friendly path instead of silently swallowing the problem
  • avoids continuing execution after state may already be invalid

So the goal here is not to remove that behavior from the product globally. The goal is to prevent test runs from inheriting it.

Why This Fix Uses Several Layers

A single switch was not enough because tests in this repository cross several boundaries:

  • PowerShell test runners
  • .runsettings
  • assembly-level NUnit bootstrap
  • some test projects that do not use the shared bootstrap/config path
  • managed code
  • native code

If only one layer is changed, another entry point can still pop UI or lose useful failure information. This fix therefore applies the same policy at each boundary: test runs must never block on modal UI, and failures must still be visible in logs.

What This Change Does

1. Mark test runs explicitly as test mode

The runners now set AssertUiEnabled=false, AssertExceptionEnabled=true, and FW_TEST_MODE=1 so tests always opt into non-interactive behavior, even when invoked outside the normal .runsettings path.

2. Give native test mode absolute priority

DebugProcs.dll now treats FW_TEST_MODE=1 as an unconditional override, ahead of the registry-based AssertMessageBox setting. This matters because some developer machines may have local registry settings that are reasonable for manual debugging but unacceptable for automated tests.

3. Preserve diagnostics instead of only suppressing UI

Suppressing the popup alone would be a partial fix because it can trade a visible hang for an opaque failure. In test mode, native assertion text is mirrored to stderr, managed Debug.Fail paths are surfaced to Console.Error, and last-chance managed exceptions are logged so the reason for failure is still visible in unattended runs.

4. Fail deterministically instead of crashing unpredictably

For unobserved task exceptions, the implementation does not throw directly from TaskScheduler.UnobservedTaskException. Instead it logs the failure, marks it observed, captures it, and fails once during suite teardown so the test host fails deterministically rather than crashing from the finalizer thread.

5. Cover projects outside the shared bootstrap path

Some installer-related projects and other test assemblies do not fully rely on the shared test bootstrap/config arrangement. This PR explicitly applies the same assert-dialog suppression and last-chance exception logging behavior there as well, so the fix does not depend on one particular project layout.

Why This Is The Best Path

This is the best path because it fixes the root cause without degrading normal developer workflows.

Other approaches were weaker:

  • only removing the popup in one native call site would miss managed paths and other entry points
  • only changing .runsettings would miss tests launched outside that flow
  • globally removing assertion UI from the product would make manual debugging worse for developers
  • simply swallowing failures would avoid hangs but lose the information needed to diagnose them

The chosen approach keeps the useful desktop-debugging behavior for ordinary app runs while making tests explicitly non-interactive, fail-fast, and readable.

Design Tradeoffs

There are a few intentional tradeoffs here:

  • More plumbing, less surprise: the fix touches runner, config, assembly bootstrap, and native assertion code because that is what complete coverage requires in a mixed native/managed desktop codebase.
  • Test-only behavior, not global behavior: FW_TEST_MODE scopes the behavior to automated runs instead of changing the product’s debugging experience for everyone.
  • Visible failure over silent continuation: when an assertion or last-chance exception occurs during tests, the design prefers clear console output and deterministic failure rather than quiet continuation.
  • Deterministic teardown failure over finalizer-thread throw: this is slightly more code, but it is much safer and easier to understand in CI logs.

Validation

  • ./build.ps1
  • ./test.ps1
  • ./test.ps1 -Native

Additional Validation

  • ./test.ps1 -TestProject "Src/InstallValidator/InstallValidatorTests/InstallValidatorTests.csproj"
  • ./test.ps1 -Native -TestProject TestGeneric
  • ./test.ps1 -Native -TestProject TestViews

Notes

  • managed test suite passed across 43 test assemblies
  • native test executables passed: testGenericLib.exe and TestViews.exe

This change is Reviewable

Copilot AI review requested due to automatic review settings March 6, 2026 20:07
@johnml1135 johnml1135 changed the title No Popups from test branches No Popups from test crashes Mar 6, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to prevent modal popups during managed and native test runs by enforcing “test mode” assertion behavior, and by ensuring assertion/exception details are visible in console output so CI fails fast and diagnosably.

Changes:

  • Add FW_TEST_MODE=1 to managed/native test runners and runsettings to bypass assertion dialog behavior (including registry overrides).
  • Introduce NUnit assembly-level attributes to suppress assert dialogs and log last-chance exceptions to console for projects that don’t use the shared bootstrap.
  • Update native test project includes to build with the DebugProcs dependency and disable assertion dialogs in testGeneric.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test.ps1 Sets FW_TEST_MODE for managed test runs outside .runsettings.
Test.runsettings Adds FW_TEST_MODE to test environment variables.
Src/InstallValidator/InstallerArtifactsTests/TestAssemblyInfo.cs Adds assembly-level unhandled-exception logging for installer artifact tests.
Src/InstallValidator/InstallerArtifactsTests/InstallerArtifactsTests.csproj Links AppForTests.config and compiles the logging attribute for tests opting out of shared bootstrap.
Src/InstallValidator/InstallValidatorTests/TestAssemblyInfo.cs Adds assembly-level unhandled-exception logging for installer validator tests.
Src/InstallValidator/InstallValidatorTests/InstallValidatorTests.csproj Compiles the logging attribute for tests opting out of shared bootstrap.
Src/Generic/Test/testGeneric.cpp Disables native assert dialogs during global test setup.
Src/Generic/Test/TestGeneric.vcxproj Fixes include paths for DebugProcs dependency.
Src/DebugProcs/DebugProcs.cpp Adds FW_TEST_MODE override for assert-dialog behavior; mirrors asserts to stderr.
Src/Common/FwUtils/FwUtilsTests/Attributes/SuppressAssertDialogsAttribute.cs New NUnit attribute to suppress assert UI and mirror trace/assert output to console.
Src/Common/FwUtils/FwUtilsTests/Attributes/LogUnhandledExceptionsAttribute.cs New NUnit attribute to log last-chance exceptions to console.
Src/Common/FwUtils/FwUtilsTests/Attributes/HandleApplicationThreadExceptionAttribute.cs Writes WinForms thread exceptions to console before rethrowing.
Src/AssemblyInfoForUiIndependentTests.cs Enables last-chance exception logging for UI-independent test assemblies.
Src/AssemblyInfoForTests.cs Enables last-chance exception logging and assert-dialog suppression for most test assemblies.
Src/AppForTests.config Adds ConsoleTraceListener to mirror trace output to console.
Build/scripts/Invoke-CppTest.ps1 Sets FW_TEST_MODE for native test runner execution.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

NUnit Tests

    1 files  ±0      1 suites  ±0   5m 57s ⏱️ +8s
4 082 tests +8  4 011 ✅ +8  71 💤 ±0  0 ❌ ±0 
4 091 runs  +8  4 020 ✅ +8  71 💤 ±0  0 ❌ ±0 

Results for commit 3374c5d. ± Comparison against base commit b6ac397.

♻️ This comment has been updated with latest results.

@johnml1135 johnml1135 force-pushed the no-popups-from-test-crashes branch from 917324f to 88c7070 Compare March 13, 2026 13:55
@johnml1135
Copy link
Contributor Author

This fix changes LogUnhandledExceptionsAttribute so unobserved task exceptions no longer throw from TaskScheduler.UnobservedTaskException.

Why this was needed:

  • The attribute summary says it is a logging-only hook, but the previous implementation threw UnobservedTaskExceptionLoggedException from the event handler.
  • On .NET Framework 4.5+ / net48, unobserved task exceptions are raised during GC/finalization and are swallowed by default after the event fires.
  • Throwing from inside that handler creates a new unhandled exception on the finalizer thread, which can terminate the entire test host nondeterministically and without useful NUnit attribution.

What this change does instead:

  • Log the unobserved task exception when the runtime raises the event.
  • Capture the AggregateException in a queue and mark it observed.
  • During suite teardown, force GC/finalizer passes while the handler is still attached, then fail once with a deterministic NUnit AssertionException that includes the captured exception details.

That preserves the original goals:

  • no popup/assert UI,
  • no silent loss of background task failures,
  • no random process crash from the finalizer thread.

Also added focused tests in FwUtilsTests for the new collector/failure behavior.

@johnml1135
Copy link
Contributor Author

Follow-up review fixes are now in 5bfc3fedf.

What changed:

  • applied SuppressAssertDialogs to the UI-independent shared test assembly info so HCSynthByGlossTest and HCSynthByGlossDllTest get the same assert-dialog suppression/fallback behavior as the UI test assemblies
  • kept AppForTests.config on FwTraceListener only; the earlier ConsoleTraceListener concern is no longer applicable because that listener was removed
  • preserved the earlier cleanup in SuppressAssertDialogsAttribute:
    • restore AssertUiEnabled, AssertExceptionEnabled, and FW_TEST_MODE after the suite
    • only insert the custom trace listener when one is not already present
    • log only failure/assert paths instead of mirroring all trace output
  • preserved the native DebugProcs cleanup:
    • gate stderr mirroring on FW_TEST_MODE
    • use warning-clean environment variable helpers so native builds stay green under warnings-as-errors

Validation run:

  • ./test.ps1 -TestProject "Src/Common/FwUtils/FwUtilsTests"
  • ./test.ps1 -Native -TestProject TestGeneric
  • ./test.ps1 -TestProject "Src/Utilities/HCSynthByGloss/HCSynthByGloss/HCSynthByGlossTest"
  • ./test.ps1 -TestProject "Src/Utilities/HCSynthByGloss/HCSynthByGlossDll/HCSynthByGlossDllTest"

@johnml1135
Copy link
Contributor Author

Summary

FieldWorks tests were still inheriting interactive desktop assertion behavior from both native and managed code paths, so some failures could block the run with modal popups instead of surfacing as ordinary test failures. Those popups exist for a legitimate reason in normal developer debugging: they force attention, allow debugger attachment, and prevent silent continuation after a serious assertion. This change keeps that interactive behavior for normal app runs, but makes test runs explicitly non-interactive, fail-fast, and diagnosable across the managed runner, native runner, shared test bootstrap, and projects that opt out of the shared bootstrap.

What Was Happening

This PR addresses a class of test failures where the process did not simply fail the test and continue. Instead, some failures flowed through existing desktop-era assertion and exception infrastructure that was designed for an interactive developer sitting at the machine.

In practice there were two main problem paths:

  • native C++ assertions could route through DebugProcs.dll and show a Windows Abort/Retry/Ignore message box
  • managed assertions and last-chance exceptions could surface through trace listeners or WinForms exception hooks rather than as a clean, visible test failure

That behavior is acceptable for interactive local debugging, but it is the wrong default for unattended test runs and CI because a popup can block the entire run indefinitely.

Why Popups Exist At All

The popup behavior is not accidental. FieldWorks is a long-lived Windows desktop application with native and managed components, and modal assertion UI is useful during manual debugging because it:

  • immediately stops the developer on a serious failure
  • offers a debugger-friendly path instead of silently swallowing the problem
  • avoids continuing execution after state may already be invalid

So the goal here is not to remove that behavior from the product globally. The goal is to prevent test runs from inheriting it.

Why This Fix Uses Several Layers

A single switch was not enough because tests in this repository cross several boundaries:

  • PowerShell test runners
  • .runsettings
  • assembly-level NUnit bootstrap
  • some test projects that do not use the shared bootstrap/config path
  • managed code
  • native code

If only one layer is changed, another entry point can still pop UI or lose useful failure information. This fix therefore applies the same policy at each boundary: test runs must never block on modal UI, and failures must still be visible in logs.

What This Change Does

1. Mark test runs explicitly as test mode

The runners now set AssertUiEnabled=false, AssertExceptionEnabled=true, and FW_TEST_MODE=1 so tests always opt into non-interactive behavior, even when invoked outside the normal .runsettings path.

2. Give native test mode absolute priority

DebugProcs.dll now treats FW_TEST_MODE=1 as an unconditional override, ahead of the registry-based AssertMessageBox setting. This matters because some developer machines may have local registry settings that are reasonable for manual debugging but unacceptable for automated tests.

3. Preserve diagnostics instead of only suppressing UI

Suppressing the popup alone would be a partial fix because it can trade a visible hang for an opaque failure. In test mode, native assertion text is mirrored to stderr, managed Debug.Fail paths are surfaced to Console.Error, and last-chance managed exceptions are logged so the reason for failure is still visible in unattended runs.

4. Fail deterministically instead of crashing unpredictably

For unobserved task exceptions, the implementation does not throw directly from TaskScheduler.UnobservedTaskException. Instead it logs the failure, marks it observed, captures it, and fails once during suite teardown so the test host fails deterministically rather than crashing from the finalizer thread.

5. Cover projects outside the shared bootstrap path

Some installer-related projects and other test assemblies do not fully rely on the shared test bootstrap/config arrangement. This PR explicitly applies the same assert-dialog suppression and last-chance exception logging behavior there as well, so the fix does not depend on one particular project layout.

Why This Is The Best Path

This is the best path because it fixes the root cause without degrading normal developer workflows.

Other approaches were weaker:

  • only removing the popup in one native call site would miss managed paths and other entry points
  • only changing .runsettings would miss tests launched outside that flow
  • globally removing assertion UI from the product would make manual debugging worse for developers
  • simply swallowing failures would avoid hangs but lose the information needed to diagnose them

The chosen approach keeps the useful desktop-debugging behavior for ordinary app runs while making tests explicitly non-interactive, fail-fast, and readable.

Design Tradeoffs

There are a few intentional tradeoffs here:

  • More plumbing, less surprise: the fix touches runner, config, assembly bootstrap, and native assertion code because that is what complete coverage requires in a mixed native/managed desktop codebase.
  • Test-only behavior, not global behavior: FW_TEST_MODE scopes the behavior to automated runs instead of changing the product’s debugging experience for everyone.
  • Visible failure over silent continuation: when an assertion or last-chance exception occurs during tests, the design prefers clear console output and deterministic failure rather than quiet continuation.
  • Deterministic teardown failure over finalizer-thread throw: this is slightly more code, but it is much safer and easier to understand in CI logs.

Validation

  • ./build.ps1
  • ./test.ps1
  • ./test.ps1 -Native

Additional Validation

  • ./test.ps1 -TestProject "Src/InstallValidator/InstallValidatorTests/InstallValidatorTests.csproj"
  • ./test.ps1 -Native -TestProject TestGeneric
  • ./test.ps1 -Native -TestProject TestViews

Notes

  • managed test suite passed across 43 test assemblies
  • native test executables passed: testGenericLib.exe and TestViews.exe

@johnml1135 johnml1135 force-pushed the no-popups-from-test-crashes branch from 41c6522 to 3374c5d Compare March 20, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants