Skip to content

Commit 5bfc3fe

Browse files
johnml1135johnml1135
committed
Finish test exception cleanup follow-ups
1 parent 4ec81a0 commit 5bfc3fe

4 files changed

Lines changed: 124 additions & 65 deletions

File tree

‎Src/AppForTests.config‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
1-
<?xml version="1.0" encoding="utf-8" ?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<configuration>
3-
<system.diagnostics>
4-
<trace autoflush="false" indentsize="4">
5-
<listeners>
6-
<clear/>
7-
<add name="ConsoleTraceListener" type="System.Diagnostics.ConsoleTraceListener"/>
8-
<add name="FwTraceListener" type="SIL.LCModel.Utils.EnvVarTraceListener, SIL.LCModel.Utils, Version=11.0.0.0, Culture=neutral"
9-
initializeData="assertuienabled='false' assertexceptionenabled='true' logfilename='%temp%/asserts.log'"/>
10-
</listeners>
11-
</trace>
12-
</system.diagnostics>
13-
<runtime>
14-
<generatePublisherEvidence enabled="false" />
15-
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
16-
<!-- Binding redirects are auto-generated at build time
17-
(AutoGenerateBindingRedirects=true in Directory.Build.props).
18-
Do NOT add manual redirects here; they will drift from CPM versions. -->
19-
</assemblyBinding>
20-
</runtime>
3+
<system.diagnostics>
4+
<trace autoflush="false" indentsize="4">
5+
<listeners>
6+
<clear />
7+
<add
8+
name="FwTraceListener"
9+
type="SIL.LCModel.Utils.EnvVarTraceListener, SIL.LCModel.Utils, Version=11.0.0.0, Culture=neutral"
10+
initializeData="assertuienabled='false' assertexceptionenabled='true' logfilename='%temp%/asserts.log'" />
11+
</listeners>
12+
</trace>
13+
</system.diagnostics>
14+
<runtime>
15+
<generatePublisherEvidence enabled="false" />
16+
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
17+
<!-- Binding redirects are auto-generated at build time
18+
(AutoGenerateBindingRedirects=true in Directory.Build.props).
19+
Do NOT add manual redirects here; they will drift from CPM versions. -->
20+
</assemblyBinding>
21+
</runtime>
2122
</configuration>

‎Src/AssemblyInfoForUiIndependentTests.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
// Log last-chance managed exceptions to console output before process termination.
1414
[assembly: LogUnhandledExceptions]
1515

16+
// Suppress all assertion dialog boxes (native + managed) regardless of config file coverage
17+
[assembly: SuppressAssertDialogs]
18+
1619
// Cleanup all singletons after running tests
1720
[assembly: CleanupSingletons]
1821

‎Src/Common/FwUtils/FwUtilsTests/Attributes/SuppressAssertDialogsAttribute.cs‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ namespace SIL.FieldWorks.Common.FwUtils.Attributes
2222
public class SuppressAssertDialogsAttribute : TestActionAttribute
2323
{
2424
private ConsoleErrorTraceListener m_listener;
25+
private string m_previousAssertUiEnabled;
26+
private string m_previousAssertExceptionEnabled;
27+
private string m_previousTestMode;
2528

2629
/// <summary/>
2730
public override ActionTargets Targets => ActionTargets.Suite;
@@ -33,22 +36,29 @@ public override void BeforeTest(ITest test)
3336

3437
// Force environment variables that control native (DebugProcs.dll) and
3538
// managed (EnvVarTraceListener) assertion behavior.
39+
m_previousAssertUiEnabled = Environment.GetEnvironmentVariable("AssertUiEnabled");
40+
m_previousAssertExceptionEnabled = Environment.GetEnvironmentVariable(
41+
"AssertExceptionEnabled"
42+
);
43+
m_previousTestMode = Environment.GetEnvironmentVariable("FW_TEST_MODE");
44+
3645
Environment.SetEnvironmentVariable("AssertUiEnabled", "false");
3746
Environment.SetEnvironmentVariable("AssertExceptionEnabled", "true");
3847
Environment.SetEnvironmentVariable("FW_TEST_MODE", "1");
3948

4049
// If EnvVarTraceListener is already installed (via AppForTests.config), keep
41-
// its assert-to-exception and file logging behavior, but still mirror output
42-
// to the console. Otherwise, our listener is responsible for failing tests.
50+
// its assert-to-exception and file logging behavior. Otherwise, our listener
51+
// is responsible for converting Debug.Fail into a test failure.
4352
bool hasEnvVarListener = false;
53+
bool hasConsoleErrorListener = false;
4454
foreach (TraceListener listener in Trace.Listeners)
4555
{
4656
// Check by type name to avoid a hard dependency on SIL.LCModel.Utils.
4757
if (listener.GetType().Name == "EnvVarTraceListener")
48-
{
4958
hasEnvVarListener = true;
50-
break;
51-
}
59+
60+
if (listener is ConsoleErrorTraceListener)
61+
hasConsoleErrorListener = true;
5262
}
5363

5464
// Suppress the DefaultTraceListener dialog even when another listener is
@@ -62,8 +72,11 @@ public override void BeforeTest(ITest test)
6272
}
6373
}
6474

65-
m_listener = new ConsoleErrorTraceListener(throwOnFail: !hasEnvVarListener);
66-
Trace.Listeners.Insert(0, m_listener);
75+
if (!hasConsoleErrorListener)
76+
{
77+
m_listener = new ConsoleErrorTraceListener(throwOnFail: !hasEnvVarListener);
78+
Trace.Listeners.Insert(0, m_listener);
79+
}
6780
}
6881

6982
/// <summary/>
@@ -75,12 +88,23 @@ public override void AfterTest(ITest test)
7588
m_listener = null;
7689
}
7790

91+
Environment.SetEnvironmentVariable("AssertUiEnabled", m_previousAssertUiEnabled);
92+
Environment.SetEnvironmentVariable(
93+
"AssertExceptionEnabled",
94+
m_previousAssertExceptionEnabled
95+
);
96+
Environment.SetEnvironmentVariable("FW_TEST_MODE", m_previousTestMode);
97+
98+
m_previousAssertUiEnabled = null;
99+
m_previousAssertExceptionEnabled = null;
100+
m_previousTestMode = null;
101+
78102
base.AfterTest(test);
79103
}
80104
}
81105

82106
/// <summary>
83-
/// Mirrors debug trace output to Console.Error and optionally converts
107+
/// Writes debug failure details to Console.Error and optionally converts
84108
/// Debug.Fail/failed Debug.Assert calls into exceptions.
85109
/// </summary>
86110
internal class ConsoleErrorTraceListener : TraceListener
@@ -102,17 +126,9 @@ public override void Fail(string message, string detailMessage)
102126
WriteFailure(message, detailMessage);
103127
}
104128

105-
public override void Write(string message)
106-
{
107-
Console.Error.Write(message);
108-
Console.Error.Flush();
109-
}
129+
public override void Write(string message) { }
110130

111-
public override void WriteLine(string message)
112-
{
113-
Console.Error.WriteLine(message);
114-
Console.Error.Flush();
115-
}
131+
public override void WriteLine(string message) { }
116132

117133
private void WriteFailure(string message, string detailMessage)
118134
{
@@ -136,8 +152,8 @@ private void WriteFailure(string message, string detailMessage)
136152
public class AssertionDialogException : Exception
137153
{
138154
public AssertionDialogException(string message)
139-
: base($"Debug.Fail/Assert fired during test (would have shown a modal dialog):\n{message}")
140-
{
141-
}
155+
: base(
156+
$"Debug.Fail/Assert fired during test (would have shown a modal dialog):\n{message}"
157+
) { }
142158
}
143159
}

‎Src/DebugProcs/DebugProcs.cpp‎

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ Last reviewed:
1919
#include <cstdlib>
2020
#include <unistd.h>
2121
#endif
22+
#include <stdlib.h>
23+
#if defined(WIN32) || defined(WIN64)
24+
#include <string.h>
25+
#else
26+
#include <strings.h>
27+
#endif
2228
#include <stdio.h>
2329
#include <assert.h>
2430
#if defined(WIN32) || defined(WIN64)
@@ -38,6 +44,46 @@ void APIENTRY DefWarnProc(const char * pszExp, const char * pszFile, int nLine,
3844
void APIENTRY DefAssertProc(const char * pszExp, const char * pszFile, int nLine, HMODULE hmod);
3945
bool GetShowAssertMessageBox();
4046

47+
static bool TryGetEnvironmentVariableValue(const char * pszName, char ** ppszValue)
48+
{
49+
#if defined(WIN32) || defined(WIN64)
50+
size_t cchValue = 0;
51+
return _dupenv_s(ppszValue, &cchValue, pszName) == 0 && *ppszValue != NULL;
52+
#else
53+
*ppszValue = getenv(pszName);
54+
return *ppszValue != NULL;
55+
#endif
56+
}
57+
58+
static void FreeEnvironmentVariableValue(char * pszValue)
59+
{
60+
#if defined(WIN32) || defined(WIN64)
61+
free(pszValue);
62+
#else
63+
(void)pszValue;
64+
#endif
65+
}
66+
67+
static bool EqualsIgnoreCase(const char * pszLeft, const char * pszRight)
68+
{
69+
#if defined(WIN32) || defined(WIN64)
70+
return _stricmp(pszLeft, pszRight) == 0;
71+
#else
72+
return strcasecmp(pszLeft, pszRight) == 0;
73+
#endif
74+
}
75+
76+
static bool IsTestModeEnabled()
77+
{
78+
char * pTestMode = NULL;
79+
if (!TryGetEnvironmentVariableValue("FW_TEST_MODE", &pTestMode))
80+
return false;
81+
82+
const bool fIsTestMode = strcmp(pTestMode, "1") == 0;
83+
FreeEnvironmentVariableValue(pTestMode);
84+
return fIsTestMode;
85+
}
86+
4187
typedef void (__stdcall * _DBG_REPORT_HOOK)(int, char *);
4288
typedef int (__stdcall * _DBG_DISPLAYMSGBOX_HOOK)(char *);
4389

@@ -291,22 +337,10 @@ extern "C" __declspec(dllexport) int APIENTRY DebugProcsExit(void)
291337
----------------------------------------------------------------------------------------------*/
292338
bool GetShowAssertMessageBox()
293339
{
294-
// getenv is deprecated on Windows
295-
#if defined(WIN32) || defined(WIN64)
296-
#pragma warning(push)
297-
#pragma warning(disable: 4996)
298-
299-
// Windows doesn't know strcasecmp, it calls it stricmp instead...
300-
#ifndef strcasecmp
301-
#define strcasecmp stricmp
302-
#endif
303-
#endif // WIN32
304-
305340
// FW_TEST_MODE is an unconditional override set by test runners.
306341
// It takes priority over both the registry and AssertUiEnabled so that
307342
// developer machines with the registry key set never pop dialogs during tests.
308-
const char* pTestMode = getenv("FW_TEST_MODE");
309-
if (pTestMode && strcasecmp(pTestMode, "1") == 0)
343+
if (IsTestModeEnabled())
310344
return false;
311345

312346
#if defined(WIN32) || defined(WIN64)
@@ -324,14 +358,16 @@ bool GetShowAssertMessageBox()
324358
return fShowAssertMessageBox ? true : false; // otherwise we get a performance warning
325359
}
326360
#endif // WIN32
327-
const char* pEnvVar = getenv("AssertUiEnabled");
328-
return !pEnvVar ||
329-
strcasecmp(pEnvVar, "0") != 0 &&
330-
strcasecmp(pEnvVar, "false") != 0 &&
331-
strcasecmp(pEnvVar, "no") != 0;
332-
#if defined(WIN32) || defined(WIN64)
333-
#pragma warning(pop)
334-
#endif // WIN32
361+
char * pEnvVar = NULL;
362+
if (!TryGetEnvironmentVariableValue("AssertUiEnabled", &pEnvVar))
363+
return true;
364+
365+
const bool fShowAssertMessageBox =
366+
!EqualsIgnoreCase(pEnvVar, "0") &&
367+
!EqualsIgnoreCase(pEnvVar, "false") &&
368+
!EqualsIgnoreCase(pEnvVar, "no");
369+
FreeEnvironmentVariableValue(pEnvVar);
370+
return fShowAssertMessageBox;
335371
}
336372

337373
/*----------------------------------------------------------------------------------------------
@@ -591,10 +627,13 @@ void __cdecl SilAssert (
591627
else
592628
OutputDebugString(assertbuf);
593629

594-
// Always mirror assertion text to the process error stream so test runners,
595-
// humans, and automation can see the failure details without a debugger.
596-
fprintf(stderr, "%s\n", assertbuf);
597-
fflush(stderr);
630+
if (IsTestModeEnabled())
631+
{
632+
// In test mode, mirror assertion text to stderr so unattended runs capture
633+
// the failure details without depending on a debugger or modal UI.
634+
fprintf(stderr, "%s\n", assertbuf);
635+
fflush(stderr);
636+
}
598637

599638
// NOTE: this method is intented to be used by unmanaged apps only;
600639
// managed apps should use DebugProcs.AssertProc in DebugProcs.cs

0 commit comments

Comments
 (0)