Conversation
There was a problem hiding this comment.
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=1to 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. |
Src/Common/FwUtils/FwUtilsTests/Attributes/SuppressAssertDialogsAttribute.cs
Outdated
Show resolved
Hide resolved
Src/Common/FwUtils/FwUtilsTests/Attributes/SuppressAssertDialogsAttribute.cs
Outdated
Show resolved
Hide resolved
Src/Common/FwUtils/FwUtilsTests/Attributes/LogUnhandledExceptionsAttribute.cs
Outdated
Show resolved
Hide resolved
Src/Common/FwUtils/FwUtilsTests/Attributes/SuppressAssertDialogsAttribute.cs
Show resolved
Hide resolved
917324f to
88c7070
Compare
|
This fix changes Why this was needed:
What this change does instead:
That preserves the original goals:
Also added focused tests in |
|
Follow-up review fixes are now in What changed:
Validation run:
|
SummaryFieldWorks 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 HappeningThis 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:
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 AllThe 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:
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 LayersA single switch was not enough because tests in this repository cross several boundaries:
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 Does1. Mark test runs explicitly as test modeThe runners now set 2. Give native test mode absolute priority
3. Preserve diagnostics instead of only suppressing UISuppressing 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 4. Fail deterministically instead of crashing unpredictablyFor unobserved task exceptions, the implementation does not throw directly from 5. Cover projects outside the shared bootstrap pathSome 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 PathThis is the best path because it fixes the root cause without degrading normal developer workflows. Other approaches were weaker:
The chosen approach keeps the useful desktop-debugging behavior for ordinary app runs while making tests explicitly non-interactive, fail-fast, and readable. Design TradeoffsThere are a few intentional tradeoffs here:
Validation
Additional Validation
Notes
|
41c6522 to
3374c5d
Compare
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:
DebugProcs.dlland show a WindowsAbort/Retry/Ignoremessage boxThat 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:
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:
.runsettingsIf 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, andFW_TEST_MODE=1so tests always opt into non-interactive behavior, even when invoked outside the normal.runsettingspath.2. Give native test mode absolute priority
DebugProcs.dllnow treatsFW_TEST_MODE=1as an unconditional override, ahead of the registry-basedAssertMessageBoxsetting. 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, managedDebug.Failpaths are surfaced toConsole.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:
.runsettingswould miss tests launched outside that flowThe 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:
FW_TEST_MODEscopes the behavior to automated runs instead of changing the product’s debugging experience for everyone.Validation
./build.ps1./test.ps1./test.ps1 -NativeAdditional Validation
./test.ps1 -TestProject "Src/InstallValidator/InstallValidatorTests/InstallValidatorTests.csproj"./test.ps1 -Native -TestProject TestGeneric./test.ps1 -Native -TestProject TestViewsNotes
testGenericLib.exeandTestViews.exeThis change is