From 37961b9eacd3824a5e8ee6838fcf8f69982feb4a Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Wed, 8 Apr 2026 13:21:15 -0600 Subject: [PATCH] feat: Enhance device testing and environment handling with new interfaces and fakes --- .../ICrestronEnvironment.cs | 7 + .../Devices/DeviceTests.cs | 323 ++++++++++++++++++ .../Fakes/CrestronEnvironmentFakes.cs | 3 + .../PepperDash.Core.Tests.csproj | 4 + src/PepperDash.Core.Tests/TestInitializer.cs | 29 ++ .../Adapters/CrestronEnvironmentAdapter.cs | 3 + src/PepperDash.Core/Device.cs | 8 +- src/PepperDash.Core/Logging/Debug.cs | 44 ++- .../Logging/DebugWebsocketSink.cs | 15 +- .../Messengers/DeviceStateMessageBase.cs | 14 - .../Messengers/MessengerBase.cs | 4 - 11 files changed, 421 insertions(+), 33 deletions(-) create mode 100644 src/PepperDash.Core.Tests/Devices/DeviceTests.cs create mode 100644 src/PepperDash.Core.Tests/TestInitializer.cs diff --git a/src/PepperDash.Core.Abstractions/ICrestronEnvironment.cs b/src/PepperDash.Core.Abstractions/ICrestronEnvironment.cs index c31fb3f6..1e73c83f 100644 --- a/src/PepperDash.Core.Abstractions/ICrestronEnvironment.cs +++ b/src/PepperDash.Core.Abstractions/ICrestronEnvironment.cs @@ -29,6 +29,13 @@ public interface ICrestronEnvironment /// Gets the application root directory path. string GetApplicationRootDirectory(); + + /// + /// Returns true when running on real Crestron hardware. + /// Returns false in test / dev environments so that SDK-specific + /// sinks and enrichers can be safely skipped. + /// + bool IsHardwareRuntime { get; } } /// diff --git a/src/PepperDash.Core.Tests/Devices/DeviceTests.cs b/src/PepperDash.Core.Tests/Devices/DeviceTests.cs new file mode 100644 index 00000000..2450e401 --- /dev/null +++ b/src/PepperDash.Core.Tests/Devices/DeviceTests.cs @@ -0,0 +1,323 @@ +using FluentAssertions; +using PepperDash.Core; +using Xunit; + +namespace PepperDash.Core.Tests.Devices; + +/// +/// Tests for — the base class for all PepperDash devices. +/// These run without Crestron hardware; Debug is initialized with fakes via TestInitializer. +/// +public class DeviceTests +{ + // ----------------------------------------------------------------------- + // Construction + // ----------------------------------------------------------------------- + + [Fact] + public void Constructor_SingleArg_SetsKey() + { + var device = new Device("my-device"); + + device.Key.Should().Be("my-device"); + } + + [Fact] + public void Constructor_SingleArg_SetsNameToEmpty() + { + var device = new Device("my-device"); + + device.Name.Should().BeEmpty(); + } + + [Fact] + public void Constructor_TwoArg_SetsKeyAndName() + { + var device = new Device("my-device", "My Device"); + + device.Key.Should().Be("my-device"); + device.Name.Should().Be("My Device"); + } + + [Fact] + public void Constructor_KeyWithDot_StillSetsKey() + { + // The dot triggers a debug log warning but must not prevent construction. + var device = new Device("parent.child"); + + device.Key.Should().Be("parent.child"); + } + + // ----------------------------------------------------------------------- + // ToString + // ----------------------------------------------------------------------- + + [Fact] + public void ToString_WithName_FormatsKeyDashName() + { + var device = new Device("cam-01", "Front Camera"); + + device.ToString().Should().Be("cam-01 - Front Camera"); + } + + [Fact] + public void ToString_WithoutName_UsesDashPlaceholder() + { + var device = new Device("cam-01"); + + device.ToString().Should().Be("cam-01 - ---"); + } + + // ----------------------------------------------------------------------- + // DefaultDevice + // ----------------------------------------------------------------------- + + [Fact] + public void DefaultDevice_IsNotNull() + { + Device.DefaultDevice.Should().NotBeNull(); + } + + [Fact] + public void DefaultDevice_HasKeyDefault() + { + Device.DefaultDevice.Key.Should().Be("Default"); + } + + // ----------------------------------------------------------------------- + // CustomActivate / Activate / Deactivate / Initialize + // ----------------------------------------------------------------------- + + [Fact] + public void CustomActivate_DefaultReturnTrue() + { + var device = new TestDevice("d1"); + + device.CallCustomActivate().Should().BeTrue(); + } + + [Fact] + public void Deactivate_DefaultReturnsTrue() + { + var device = new Device("d1"); + + device.Deactivate().Should().BeTrue(); + } + + [Fact] + public void Activate_CallsCustomActivate_AndReturnsItsResult() + { + var stub = new ActivateTrackingDevice("d1", result: false); + + stub.Activate().Should().BeFalse(); + stub.CustomActivateCalled.Should().BeTrue(); + } + + [Fact] + public void Activate_TrueWhenCustomActivateReturnsTrue() + { + var stub = new ActivateTrackingDevice("d1", result: true); + + stub.Activate().Should().BeTrue(); + } + + [Fact] + public void Initialize_DoesNotThrow() + { + var device = new TestDevice("d1"); + var act = () => device.CallInitialize(); + + act.Should().NotThrow(); + } + + // ----------------------------------------------------------------------- + // PreActivate + // ----------------------------------------------------------------------- + + [Fact] + public void PreActivate_NoActions_DoesNotThrow() + { + var device = new TestDevice("d1"); + var act = () => device.PreActivate(); + + act.Should().NotThrow(); + } + + [Fact] + public void PreActivate_RunsRegisteredActionsInOrder() + { + var device = new TestDevice("d1"); + var order = new List(); + + device.AddPreActivationAction(() => order.Add(1)); + device.AddPreActivationAction(() => order.Add(2)); + device.AddPreActivationAction(() => order.Add(3)); + + device.PreActivate(); + + order.Should().Equal(1, 2, 3); + } + + [Fact] + public void PreActivate_ContinuesAfterFaultingAction() + { + var device = new TestDevice("d1"); + var reached = false; + + device.AddPreActivationAction(() => throw new InvalidOperationException("boom")); + device.AddPreActivationAction(() => reached = true); + + var act = () => device.PreActivate(); + + act.Should().NotThrow("exceptions in individual actions must be caught internally"); + reached.Should().BeTrue("actions after a faulting action must still run"); + } + + // ----------------------------------------------------------------------- + // PostActivate + // ----------------------------------------------------------------------- + + [Fact] + public void PostActivate_NoActions_DoesNotThrow() + { + var device = new TestDevice("d1"); + var act = () => device.PostActivate(); + + act.Should().NotThrow(); + } + + [Fact] + public void PostActivate_RunsRegisteredActionsInOrder() + { + var device = new TestDevice("d1"); + var order = new List(); + + device.AddPostActivationAction(() => order.Add(1)); + device.AddPostActivationAction(() => order.Add(2)); + + device.PostActivate(); + + order.Should().Equal(1, 2); + } + + [Fact] + public void PostActivate_ContinuesAfterFaultingAction() + { + var device = new TestDevice("d1"); + var reached = false; + + device.AddPostActivationAction(() => throw new Exception("boom")); + device.AddPostActivationAction(() => reached = true); + + var act = () => device.PostActivate(); + + act.Should().NotThrow(); + reached.Should().BeTrue(); + } + + // ----------------------------------------------------------------------- + // Pre and Post actions are independent lists + // ----------------------------------------------------------------------- + + [Fact] + public void PreActivationActions_DoNotRunOnPostActivate() + { + var device = new TestDevice("d1"); + var preRan = false; + + device.AddPreActivationAction(() => preRan = true); + device.PostActivate(); + + preRan.Should().BeFalse(); + } + + [Fact] + public void PostActivationActions_DoNotRunOnPreActivate() + { + var device = new TestDevice("d1"); + var postRan = false; + + device.AddPostActivationAction(() => postRan = true); + device.PreActivate(); + + postRan.Should().BeFalse(); + } + + // ----------------------------------------------------------------------- + // OnFalse + // ----------------------------------------------------------------------- + + [Fact] + public void OnFalse_FiresAction_WhenBoolIsFalse() + { + var device = new Device("d1"); + var fired = false; + + device.OnFalse(false, () => fired = true); + + fired.Should().BeTrue(); + } + + [Fact] + public void OnFalse_DoesNotFireAction_WhenBoolIsTrue() + { + var device = new Device("d1"); + var fired = false; + + device.OnFalse(true, () => fired = true); + + fired.Should().BeFalse(); + } + + [Fact] + public void OnFalse_DoesNotFireAction_ForNonBoolType() + { + var device = new Device("d1"); + var fired = false; + + device.OnFalse("not a bool", () => fired = true); + device.OnFalse(0, () => fired = true); + device.OnFalse(null!, () => fired = true); + + fired.Should().BeFalse(); + } + + // ----------------------------------------------------------------------- + // Helpers + // ----------------------------------------------------------------------- + + /// + /// Exposes protected Device members so test methods can call them directly. + /// + private class TestDevice : Device + { + public TestDevice(string key) : base(key) { } + public TestDevice(string key, string name) : base(key, name) { } + + public void AddPreActivationAction(Action act) => base.AddPreActivationAction(act); + public void AddPostActivationAction(Action act) => base.AddPostActivationAction(act); + public bool CallCustomActivate() => base.CustomActivate(); + public void CallInitialize() => base.Initialize(); + } + + /// + /// Records whether CustomActivate was invoked and returns a configured result. + /// Used to verify Activate() correctly delegates to CustomActivate(). + /// + private sealed class ActivateTrackingDevice : Device + { + private readonly bool _result; + public bool CustomActivateCalled { get; private set; } + + public ActivateTrackingDevice(string key, bool result = true) : base(key) + { + _result = result; + } + + protected override bool CustomActivate() + { + CustomActivateCalled = true; + return _result; + } + } +} diff --git a/src/PepperDash.Core.Tests/Fakes/CrestronEnvironmentFakes.cs b/src/PepperDash.Core.Tests/Fakes/CrestronEnvironmentFakes.cs index 8070f849..1792206d 100644 --- a/src/PepperDash.Core.Tests/Fakes/CrestronEnvironmentFakes.cs +++ b/src/PepperDash.Core.Tests/Fakes/CrestronEnvironmentFakes.cs @@ -19,6 +19,9 @@ public class FakeCrestronEnvironment : ICrestronEnvironment public string GetApplicationRootDirectory() => System.IO.Path.GetTempPath(); + /// + public bool IsHardwareRuntime => false; + /// Simulates a program status event for tests. public void RaiseProgramStatus(ProgramStatusEventType type) => ProgramStatusChanged?.Invoke(this, new ProgramStatusEventArgs(type)); diff --git a/src/PepperDash.Core.Tests/PepperDash.Core.Tests.csproj b/src/PepperDash.Core.Tests/PepperDash.Core.Tests.csproj index 54b74d00..9e045a89 100644 --- a/src/PepperDash.Core.Tests/PepperDash.Core.Tests.csproj +++ b/src/PepperDash.Core.Tests/PepperDash.Core.Tests.csproj @@ -21,5 +21,9 @@ + + diff --git a/src/PepperDash.Core.Tests/TestInitializer.cs b/src/PepperDash.Core.Tests/TestInitializer.cs new file mode 100644 index 00000000..6455ea59 --- /dev/null +++ b/src/PepperDash.Core.Tests/TestInitializer.cs @@ -0,0 +1,29 @@ +using System.Runtime.CompilerServices; +using PepperDash.Core.Abstractions; +using PepperDash.Core.Tests.Fakes; + +namespace PepperDash.Core.Tests; + +/// +/// Runs once before any type in this assembly is accessed. +/// Registers fake Crestron service implementations with +/// so that the Debug static constructor uses them instead of the real Crestron SDK. +/// This must remain a module initializer (not a test fixture) because the static constructor +/// fires the first time any type in PepperDash.Core is referenced — before xUnit +/// has a chance to run fixture setup code. +/// +internal static class TestInitializer +{ + [ModuleInitializer] + internal static void Initialize() + { + DebugServiceRegistration.Register( + new FakeCrestronEnvironment + { + DevicePlatform = DevicePlatform.Server, // avoids any appliance-only code paths + RuntimeEnvironment = RuntimeEnvironment.Other, // skips console command registration + }, + new NoOpCrestronConsole(), + new InMemoryCrestronDataStore()); + } +} diff --git a/src/PepperDash.Core/Adapters/CrestronEnvironmentAdapter.cs b/src/PepperDash.Core/Adapters/CrestronEnvironmentAdapter.cs index 752e9631..62002d73 100644 --- a/src/PepperDash.Core/Adapters/CrestronEnvironmentAdapter.cs +++ b/src/PepperDash.Core/Adapters/CrestronEnvironmentAdapter.cs @@ -57,6 +57,9 @@ public sealed class CrestronEnvironmentAdapter : PdCore.ICrestronEnvironment public string GetApplicationRootDirectory() => Crestron.SimplSharp.CrestronIO.Directory.GetApplicationRootDirectory(); + /// + public bool IsHardwareRuntime => true; + private static PdCore.ProgramStatusEventType MapProgramStatus(eProgramStatusEventType type) => type switch { diff --git a/src/PepperDash.Core/Device.cs b/src/PepperDash.Core/Device.cs index 70d4d97f..ff436b68 100644 --- a/src/PepperDash.Core/Device.cs +++ b/src/PepperDash.Core/Device.cs @@ -75,7 +75,7 @@ public class Device : IKeyName /// Adds a pre activation action /// /// - public void AddPreActivationAction(Action act) + protected void AddPreActivationAction(Action act) { if (_PreActivationActions == null) _PreActivationActions = new List(); @@ -89,7 +89,7 @@ public class Device : IKeyName /// /// AddPostActivationAction method /// - public void AddPostActivationAction(Action act) + protected void AddPostActivationAction(Action act) { if (_PostActivationActions == null) _PostActivationActions = new List(); @@ -156,7 +156,7 @@ public class Device : IKeyName /// /// CustomActivate method /// - public virtual bool CustomActivate() { return true; } + protected virtual bool CustomActivate() { return true; } /// /// Call to deactivate device - unlink events, etc. Overriding classes do not @@ -168,7 +168,7 @@ public class Device : IKeyName /// /// Call this method to start communications with a device. Overriding classes do not need to call base.Initialize() /// - public virtual void Initialize() + protected virtual void Initialize() { } diff --git a/src/PepperDash.Core/Logging/Debug.cs b/src/PepperDash.Core/Logging/Debug.cs index fb0483bb..97a6e93e 100644 --- a/src/PepperDash.Core/Logging/Debug.cs +++ b/src/PepperDash.Core/Logging/Debug.cs @@ -96,7 +96,7 @@ public static class Debug /// /// The name of the file containing the current debug settings. /// - public static string FileName = string.Format(@"app{0}Debug.json", InitialParametersClass.ApplicationNumber); + public static string FileName = "app0Debug.json"; // default; updated in static ctor using _environment.ApplicationNumber /// /// Debug level to set for a given program. @@ -161,6 +161,9 @@ public static class Debug IsRunningOnAppliance = _environment?.DevicePlatform == PdCore.DevicePlatform.Appliance; + // Update FileName now that _environment is available (avoids Crestron SDK ref in field initializer). + FileName = $"app{_environment?.ApplicationNumber ?? 0}Debug.json"; + _dataStore?.InitStore(); consoleDebugTimer = new Timer(defaultConsoleDebugTimeoutMin * 60000) { AutoReset = false }; @@ -180,7 +183,7 @@ public static class Debug errorLogLevelSwitch = new LoggingLevelSwitch(initialMinimumLevel: defaultErrorLogLevel); fileLoggingLevelSwitch = new LoggingLevelSwitch(initialMinimumLevel: defaultFileLogLevel); - websocketSink = new DebugWebsocketSink(new JsonFormatter(renderMessage: true)); + websocketSink = TryCreateWebsocketSink(); var appRoot = _environment?.GetApplicationRootDirectory() ?? System.IO.Path.GetTempPath(); @@ -200,7 +203,6 @@ public static class Debug .MinimumLevel.Verbose() .Enrich.FromLogContext() .WriteTo.Sink(new DebugConsoleSink(new ExpressionTemplate("[{@t:yyyy-MM-dd HH:mm:ss.fff}][{@l:u4}][{App}]{#if Key is not null}[{Key}]{#end} {@m}{#if @x is not null}\r\n{@x}{#end}")), levelSwitch: consoleLoggingLevelSwitch) - .WriteTo.Sink(websocketSink, levelSwitch: websocketLoggingLevelSwitch) .WriteTo.File(new RenderedCompactJsonFormatter(), logFilePath, rollingInterval: RollingInterval.Day, restrictedToMinimumLevel: LogEventLevel.Debug, @@ -208,8 +210,12 @@ public static class Debug levelSwitch: fileLoggingLevelSwitch ); - // Add Crestron-specific enricher and error-log sink only when running on hardware. - if (_environment != null) + // Websocket sink is null when DebugWebsocketSink failed to construct (e.g. test env). + if (websocketSink != null) + _defaultLoggerConfiguration.WriteTo.Sink(websocketSink, levelSwitch: websocketLoggingLevelSwitch); + + // Add Crestron-specific enricher and error-log sink only on real hardware. + if (_environment?.IsHardwareRuntime == true) { var errorLogTemplate = IsRunningOnAppliance ? "{@t:fff}ms [{@l:u4}]{#if Key is not null}[{Key}]{#end} {@m}{#if @x is not null}\r\n{@x}{#end}" @@ -271,8 +277,34 @@ public static class Debug { // _logger may not have been initialized yet — do not call LogError here. // _console may also be null; fall back to CrestronConsole as last resort. + // IMPORTANT: this catch block must not throw — any exception escaping a static + // constructor permanently faults the type, making the entire class unusable. try { _console?.PrintLine($"Exception in Debug static constructor: {ex.Message}\r\n{ex.StackTrace}"); } - catch { CrestronConsole.PrintLine($"Exception in Debug static constructor: {ex.Message}\r\n{ex.StackTrace}"); } + catch + { + try { CrestronConsole.PrintLine($"Exception in Debug static constructor: {ex.Message}\r\n{ex.StackTrace}"); } + catch { /* CrestronConsole unavailable (test/dev env) — swallow to keep type initializer healthy */ } + } + } + finally + { + // Guarantee _logger is never null — all Debug.Log* calls are safe even if the + // ctor failed partway through (e.g. on a dev machine without Crestron hardware). + _logger ??= new LoggerConfiguration().MinimumLevel.Fatal().CreateLogger(); + } + } + + /// Creates the WebSocket sink, returning null if construction fails in a test/dev environment. + private static DebugWebsocketSink? TryCreateWebsocketSink() + { + try + { + return new DebugWebsocketSink(new JsonFormatter(renderMessage: true)); + } + catch + { + _console?.PrintLine("DebugWebsocketSink could not be created in this environment; websocket logging disabled."); + return null; } } diff --git a/src/PepperDash.Core/Logging/DebugWebsocketSink.cs b/src/PepperDash.Core/Logging/DebugWebsocketSink.cs index 6f1aee18..52d61896 100644 --- a/src/PepperDash.Core/Logging/DebugWebsocketSink.cs +++ b/src/PepperDash.Core/Logging/DebugWebsocketSink.cs @@ -91,13 +91,18 @@ public class DebugWebsocketSink : ILogEventSink, IKeyed if (!File.Exists(CertPath)) CreateCert(); - CrestronEnvironment.ProgramStatusEventHandler += type => + try { - if (type == eProgramStatusEventType.Stopping) + CrestronEnvironment.ProgramStatusEventHandler += type => { - StopServer(); - } - }; + if (type == eProgramStatusEventType.Stopping) + StopServer(); + }; + } + catch + { + // CrestronEnvironment is not available in test / dev environments — safe to skip. + } } private static void CreateCert() diff --git a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DeviceStateMessageBase.cs b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DeviceStateMessageBase.cs index 87f19e3f..1e538a31 100644 --- a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DeviceStateMessageBase.cs +++ b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/DeviceStateMessageBase.cs @@ -8,20 +8,6 @@ namespace PepperDash.Essentials.AppServer.Messengers /// public class DeviceStateMessageBase : DeviceMessageBase { - /// - /// The interfaces implmented by the device sending the messsage - /// - [JsonProperty("interfaces")] - public List Interfaces { get; private set; } - - /// - /// Sets the interfaces implemented by the device sending the message - /// - /// - public void SetInterfaces(List interfaces) - { - Interfaces = interfaces; - } } } \ No newline at end of file diff --git a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs index 9d22803d..fbbaa5e5 100644 --- a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs +++ b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs @@ -258,8 +258,6 @@ namespace PepperDash.Essentials.AppServer.Messengers throw new ArgumentNullException("device"); } - message.SetInterfaces(_deviceInterfaces); - message.Key = _device.Key; message.Name = _device.Name; @@ -285,8 +283,6 @@ namespace PepperDash.Essentials.AppServer.Messengers { try { - deviceState.SetInterfaces(_deviceInterfaces); - deviceState.Key = _device.Key; deviceState.Name = _device.Name;