diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56117b6ba..b56c5bab8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,7 +87,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] uses: ./.github/workflows/smoke-test-create.yml with: unity-version: ${{ matrix.unity-version }} @@ -101,7 +101,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] platform: ["WebGL", "Linux"] include: - platform: WebGL @@ -205,7 +205,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] uses: ./.github/workflows/smoke-test-build-android.yml with: unity-version: ${{ matrix.unity-version }} @@ -224,7 +224,7 @@ jobs: matrix: api-level: [30, 31, 34] # last updated January 2025 init-type: ["runtime", "buildtime"] - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] smoke-test-build-ios: name: Build iOS ${{ matrix.unity-version }} Smoke Test @@ -234,7 +234,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] uses: ./.github/workflows/smoke-test-build-ios.yml with: unity-version: ${{ matrix.unity-version }} @@ -247,7 +247,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] init-type: ["runtime", "buildtime"] uses: ./.github/workflows/smoke-test-compile-ios.yml with: @@ -266,7 +266,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] # Check https://support.apple.com/en-us/HT201222 for the latest minor version for a given major one. # https://developer.apple.com/support/app-store/ shows that of all iOS devices # - `iOS 17`: 86 % @@ -287,7 +287,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] platform: ["WebGL", "Linux"] steps: - name: Checkout @@ -325,7 +325,7 @@ jobs: strategy: fail-fast: false matrix: - unity-version: ["2020", "2022", "6000"] + unity-version: ["2020", "2022", "6000", "6100"] # os: ["windows", "macos"] os: ["windows"] include: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a9cb9e9d..3e7f8e661 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Fixes + +- The SDK now waits for 'End of Frame' before capturing a screenshot. This should address any blank or malformed + screenshots previously attached to events. The SDK now also only captures one screenshot for the first error event in + each individual frame. ([#2240](https://github.com/getsentry/sentry-unity/pull/2240)) + ### Dependencies - Bump CLI from v2.47.1 to v2.50.2 ([#2245](https://github.com/getsentry/sentry-unity/pull/2245), [#2250](https://github.com/getsentry/sentry-unity/pull/2250)) diff --git a/samples/unity-of-bugs/Assets/Resources/Sentry/SentryOptions.asset b/samples/unity-of-bugs/Assets/Resources/Sentry/SentryOptions.asset index 153017e4e..831e829b4 100644 --- a/samples/unity-of-bugs/Assets/Resources/Sentry/SentryOptions.asset +++ b/samples/unity-of-bugs/Assets/Resources/Sentry/SentryOptions.asset @@ -28,10 +28,10 @@ MonoBehaviour: k__BackingField: k__BackingField: k__BackingField: 1 - k__BackingField: 0 + k__BackingField: 1 k__BackingField: 1 k__BackingField: 75 - k__BackingField: 0 + k__BackingField: 1 k__BackingField: 100 k__BackingField: 20 k__BackingField: 10 diff --git a/scripts/ci-env.ps1 b/scripts/ci-env.ps1 index 448f9c53b..a541243f3 100644 --- a/scripts/ci-env.ps1 +++ b/scripts/ci-env.ps1 @@ -19,6 +19,9 @@ switch ($name) { "unity6000" { return "6000.0.49f1" } + "unity6100" { + return "6000.1.12f1" + } Default { throw "Unkown variable '$name'" } diff --git a/scripts/smoke-test-android.ps1 b/scripts/smoke-test-android.ps1 index 4376bfa87..7d40fd837 100644 --- a/scripts/smoke-test-android.ps1 +++ b/scripts/smoke-test-android.ps1 @@ -24,7 +24,7 @@ Write-Host "#####################################################" $BuildDir = $(GetNewProjectBuildPath) $ApkFileName = "test.apk" -$ProcessName = "com.DefaultCompany.$(GetNewProjectName)" +$ProcessName = "io.sentry.unity.integrationtest" $TestActivityName = "$ProcessName/com.unity3d.player.UnityPlayerActivity" $FallBackTestActivityName = "$ProcessName/com.unity3d.player.UnityPlayerGameActivity" diff --git a/src/Sentry.Unity/ScreenshotEventProcessor.cs b/src/Sentry.Unity/ScreenshotEventProcessor.cs index a8b8e3584..57a22093a 100644 --- a/src/Sentry.Unity/ScreenshotEventProcessor.cs +++ b/src/Sentry.Unity/ScreenshotEventProcessor.cs @@ -1,64 +1,85 @@ +using System; +using System.Collections; +using System.Threading; using Sentry.Extensibility; -using Sentry.Unity.Integrations; +using Sentry.Internal; using UnityEngine; namespace Sentry.Unity; -public class ScreenshotEventProcessor : ISentryEventProcessorWithHint +public class ScreenshotEventProcessor : ISentryEventProcessor { private readonly SentryUnityOptions _options; - private readonly IApplication _application; - public ScreenshotEventProcessor(SentryUnityOptions sentryOptions) : this(sentryOptions, null) { } + private readonly ISentryMonoBehaviour _sentryMonoBehaviour; + private volatile int _isCapturingScreenshot; - internal ScreenshotEventProcessor(SentryUnityOptions sentryOptions, IApplication? application) + public ScreenshotEventProcessor(SentryUnityOptions sentryOptions) : this(sentryOptions, SentryMonoBehaviour.Instance) { } + + internal ScreenshotEventProcessor(SentryUnityOptions sentryOptions, ISentryMonoBehaviour sentryMonoBehaviour) { _options = sentryOptions; - _application = application ?? ApplicationAdapter.Instance; + _sentryMonoBehaviour = sentryMonoBehaviour; } - public SentryEvent? Process(SentryEvent @event) + public SentryEvent Process(SentryEvent @event) { + // Only ever capture one screenshot per frame + if (Interlocked.CompareExchange(ref _isCapturingScreenshot, 1, 0) == 0) + { + _sentryMonoBehaviour.StartCoroutine(CaptureScreenshotCoroutine(@event.EventId)); + } + return @event; } - public SentryEvent? Process(SentryEvent @event, SentryHint hint) + internal IEnumerator CaptureScreenshotCoroutine(SentryId eventId) { - // save event id - // wait for end of frame - // check if last id is event it - // send screenshot - - // add workitem: screentshot for ID xxx - // sdk integration checking for work: if ID got sent, follow up with screenshot + _options.LogDebug("Screenshot capture triggered. Waiting for End of Frame."); - if (!MainThreadData.IsMainThread()) - { - _options.DiagnosticLogger?.LogDebug("Screenshot capture skipped. Can't capture screenshots on other than the main thread."); - return @event; - } + // WaitForEndOfFrame does not work in headless mode so we're making it configurable for CI. + // See https://docs.unity3d.com/6000.1/Documentation/ScriptReference/WaitForEndOfFrame.html + yield return WaitForEndOfFrame(); - if (_options.BeforeCaptureScreenshotInternal?.Invoke() is not false) + try { - if (_application.IsEditor) + if (_options.BeforeCaptureScreenshotInternal?.Invoke() is false) { - _options.DiagnosticLogger?.LogInfo("Screenshot capture skipped. Capturing screenshots it not supported in the Editor"); - return @event; + yield break; } - if (Screen.width == 0 || Screen.height == 0) + var screenshotBytes = CaptureScreenshot(_options); + if (screenshotBytes.Length == 0) { - _options.DiagnosticLogger?.LogWarning("Can't capture screenshots on a screen with a resolution of '{0}x{1}'.", Screen.width, Screen.height); - } - else - { - hint.AddAttachment(SentryScreenshot.Capture(_options), "screenshot.jpg", contentType: "image/jpeg"); + _options.LogWarning("Screenshot capture returned empty data for event {0}", eventId); + yield break; } + + var attachment = new SentryAttachment( + AttachmentType.Default, + new ByteAttachmentContent(screenshotBytes), + "screenshot.jpg", + "image/jpeg"); + + _options.LogDebug("Screenshot captured for event {0}", eventId); + + CaptureAttachment(eventId, attachment); } - else + catch (Exception e) { - _options.DiagnosticLogger?.LogInfo("Screenshot capture skipped by BeforeAttachScreenshot callback."); + _options.LogError(e, "Failed to capture screenshot."); + } + finally + { + Interlocked.Exchange(ref _isCapturingScreenshot, 0); } - - return @event; } + + internal virtual byte[] CaptureScreenshot(SentryUnityOptions options) + => SentryScreenshot.Capture(options); + + internal virtual void CaptureAttachment(SentryId eventId, SentryAttachment attachment) + => (Sentry.SentrySdk.CurrentHub as Hub)?.CaptureAttachment(eventId, attachment); + + internal virtual YieldInstruction WaitForEndOfFrame() + => new WaitForEndOfFrame(); } diff --git a/src/Sentry.Unity/SentryMonoBehaviour.cs b/src/Sentry.Unity/SentryMonoBehaviour.cs index 0945d3e72..4b1ff5f88 100644 --- a/src/Sentry.Unity/SentryMonoBehaviour.cs +++ b/src/Sentry.Unity/SentryMonoBehaviour.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using Sentry.Unity.Integrations; using UnityEngine; @@ -7,6 +8,7 @@ namespace Sentry.Unity; internal interface ISentryMonoBehaviour { event Action? ApplicationResuming; + public Coroutine StartCoroutine(IEnumerator routine); } /// diff --git a/src/Sentry.Unity/SentryUnitySdk.cs b/src/Sentry.Unity/SentryUnitySdk.cs index c590cbbda..99252ee3f 100644 --- a/src/Sentry.Unity/SentryUnitySdk.cs +++ b/src/Sentry.Unity/SentryUnitySdk.cs @@ -157,7 +157,7 @@ internal static void AddIntegrations(SentryUnityOptions options) { options.AddEventProcessor(new ViewHierarchyEventProcessor(options)); } - if (options.AttachScreenshot) + if (!ApplicationAdapter.Instance.IsEditor && options.AttachScreenshot) { options.AddEventProcessor(new ScreenshotEventProcessor(options)); } diff --git a/src/sentry-dotnet b/src/sentry-dotnet index 57901f427..25a894dd4 160000 --- a/src/sentry-dotnet +++ b/src/sentry-dotnet @@ -1 +1 @@ -Subproject commit 57901f42726dcca3232fe5bee9c3f7ff41646b1f +Subproject commit 25a894dd4e295bcb486de055ca547b1eb2fc270b diff --git a/test/Scripts.Integration.Test/Editor/Builder.cs b/test/Scripts.Integration.Test/Editor/Builder.cs index e2955b439..a55adec42 100644 --- a/test/Scripts.Integration.Test/Editor/Builder.cs +++ b/test/Scripts.Integration.Test/Editor/Builder.cs @@ -22,6 +22,8 @@ public static void BuildIl2CPPPlayer(BuildTarget target, BuildTargetGroup group, EditorUserBuildSettings.selectedBuildTargetGroup = group; EditorUserBuildSettings.allowDebugging = false; PlayerSettings.SetScriptingBackend(group, ScriptingImplementation.IL2CPP); + // Making sure that the app keeps on running in the background. Linux CI is very unhappy with coroutines otherwise. + PlayerSettings.runInBackground = true; DisableUnityAudio(); DisableProgressiveLightMapper(); @@ -41,14 +43,6 @@ public static void BuildIl2CPPPlayer(BuildTarget target, BuildTargetGroup group, PlayerSettings.SetManagedStrippingLevel(group, ManagedStrippingLevel.Low); #endif - - // This is a workaround for build issues with Unity 2022.3. and newer. - // https://discussions.unity.com/t/gradle-build-issues-for-android-api-sdk-35-in-unity-2022-3lts/1502187/10 -#if UNITY_2022_3_OR_NEWER - Debug.Log("Builder: Setting Android target API level to 33"); - PlayerSettings.Android.targetSdkVersion = AndroidSdkVersions.AndroidApiLevel33; -#endif - Debug.Log("Builder: Updating BuildPlayerOptions"); var buildPlayerOptions = new BuildPlayerOptions { @@ -75,6 +69,9 @@ public static void BuildIl2CPPPlayer(BuildTarget target, BuildTargetGroup group, if (target == BuildTarget.Android) { + Debug.Log("Builder: Setting application identifier"); + PlayerSettings.SetApplicationIdentifier(BuildTargetGroup.Android, "io.sentry.unity.integrationtest"); + // Android does not support appending builds. We make sure the directory is clean var outputDir = Path.GetDirectoryName(args["buildPath"]); if (Directory.Exists(outputDir)) diff --git a/test/Scripts.Integration.Test/Scripts/OptionsConfiguration.cs b/test/Scripts.Integration.Test/Scripts/OptionsConfiguration.cs index 0f8bccf5b..ffe29a116 100644 --- a/test/Scripts.Integration.Test/Scripts/OptionsConfiguration.cs +++ b/test/Scripts.Integration.Test/Scripts/OptionsConfiguration.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using Sentry; using Sentry.Unity; using UnityEngine; @@ -37,6 +38,16 @@ public override void Configure(SentryUnityOptions options) return -2; }; + // Filtering the SmokeTester logs from the breadcrumbs here + options.AddBreadcrumbsForLogType = new Dictionary + { + { LogType.Error, true}, + { LogType.Assert, true}, + { LogType.Warning, true}, + { LogType.Log, false}, // No breadcrumbs for Debug.Log + { LogType.Exception, true}, + }; + // If an ANR triggers while the smoke test runs, the test would fail because we expect exact order of events. options.DisableAnrIntegration(); diff --git a/test/Scripts.Integration.Test/Scripts/SmokeTester.cs b/test/Scripts.Integration.Test/Scripts/SmokeTester.cs index 16becdfdf..12c649b67 100644 --- a/test/Scripts.Integration.Test/Scripts/SmokeTester.cs +++ b/test/Scripts.Integration.Test/Scripts/SmokeTester.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Collections.Concurrent; using System.Linq; @@ -23,7 +24,7 @@ private void Awake() Debug.Log("SmokeTester, awake!"); Application.quitting += () => { - // We're using this in the smoke-test-android.ps1 script to reliably detect when the tests have finished running. + // The smoke-test-android.ps1 reads this from console to reliably detect when the tests have finished running. Debug.Log("SmokeTester is quitting."); }; } @@ -37,7 +38,8 @@ public void Start() if (arg == "smoke") { - SmokeTest(); + // Running this as coroutine as we need to skip some frames to wait for screenshot capture + StartCoroutine(SmokeTestCoroutine()); } else if (arg == "hasnt-crashed") { @@ -92,92 +94,103 @@ private static string GetTestArg() internal static Func CrashedLastRun = () => -1; - public static void SmokeTest() + private IEnumerator SmokeTestCoroutine() { t.Start("SMOKE"); - try - { + #if !UNITY_EDITOR - var crashed = CrashedLastRun(); - t.Expect($"options.CrashedLastRun ({crashed}) == false (0)", crashed == 0); + var crashed = CrashedLastRun(); + t.Expect($"options.CrashedLastRun ({crashed}) == false (0)", crashed == 0); #endif - var currentMessage = 0; - t.ExpectMessage(currentMessage, "'type':'session'"); + var currentMessage = 0; + t.ExpectMessage(currentMessage, "'type':'session'"); - // Skip the session init requests (there may be multiple of them). We can't skip them by a "positive" - // because they're also repeated with standard events (in an envelope). - Debug.Log("SMOKE TEST: Skipping all session requests"); - for (; currentMessage < 10; currentMessage++) + // Skip the session init requests (there may be multiple of them). We can't skip them by a "positive" + // because they're also repeated with standard events (in an envelope). + Debug.Log("Skipping all session requests"); + for (; currentMessage < 10; currentMessage++) + { + if (t.CheckMessage(currentMessage, "'type':'transaction'")) { - if (t.CheckMessage(currentMessage, "'type':'transaction'")) - { - break; - } + break; } - Debug.Log($"SMOKE TEST: Done skipping session requests. Last one was: #{currentMessage}"); + } + Debug.Log($"Done skipping session requests. Last one was: #{currentMessage}"); - t.ExpectMessage(currentMessage, "'type':'transaction"); - t.ExpectMessage(currentMessage, "'op':'app.start'"); // startup transaction + t.ExpectMessage(currentMessage, "'type':'transaction"); + t.ExpectMessage(currentMessage, "'op':'app.start'"); // startup transaction #if !UNITY_EDITOR - t.ExpectMessage(currentMessage, "'op':'awake','description':'Main Camera.SmokeTester'"); // auto instrumentation + t.ExpectMessage(currentMessage, "'op':'awake','description':'Main Camera.SmokeTester'"); // auto instrumentation #endif - t.ExpectMessageNot(currentMessage, "'length':0"); - - var guid = Guid.NewGuid().ToString(); - Debug.LogError($"LogError(GUID)={guid}"); - currentMessage++; - - t.ExpectMessage(currentMessage, "'type':'event'"); - t.ExpectMessage(currentMessage, $"LogError(GUID)={guid}"); - // Contexts - t.ExpectMessage(currentMessage, "'type':'app',"); - t.ExpectMessage(currentMessage, "'type':'device',"); - t.ExpectMessage(currentMessage, "'type':'gpu',"); - t.ExpectMessage(currentMessage, "'type':'os',"); - t.ExpectMessage(currentMessage, "'type':'runtime',"); - t.ExpectMessage(currentMessage, "'type':'unity',"); - // User - t.ExpectMessage(currentMessage, "'user':{'id':'"); // non-null automatic ID - // Attachment - t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); - t.ExpectMessageNot(currentMessage, "'length':0"); - - SentrySdk.CaptureMessage($"CaptureMessage(GUID)={guid}"); - currentMessage++; - - t.ExpectMessage(currentMessage, "'type':'event'"); - t.ExpectMessage(currentMessage, $"CaptureMessage(GUID)={guid}"); - t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); - t.ExpectMessageNot(currentMessage, "'length':0"); - - var ex = new Exception("Exception & context test"); - AddContext(); - SentrySdk.CaptureException(ex); - t.ExpectMessage(++currentMessage, "'type':'event'"); - t.ExpectMessage(currentMessage, "'message':'crumb','type':'error','data':{'foo':'bar'},'category':'bread','level':'critical'}"); - t.ExpectMessage(currentMessage, "'message':'scope-crumb'}"); - t.ExpectMessage(currentMessage, "'extra':{'extra-key':42}"); - t.ExpectMessage(currentMessage, "'tag-key':'tag-value'"); - t.ExpectMessage(currentMessage, "'user':{'id':'user-id','username':'username','email':'email@example.com','ip_address':'::1','other':{'role':'admin'}}"); - t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); - t.ExpectMessageNot(currentMessage, "'length':0"); - - Debug.Log("SMOKE TEST: Finished checking messages."); - - t.Pass(); - } - catch (Exception ex) - { - if (t.ExitCode == 0) - { - Debug.Log($"SMOKE TEST: FAILED with exception {ex}"); - t.Exit(-1); - } - else - { - Debug.Log("SMOKE TEST: FAILED"); - } - } + t.ExpectMessageNot(currentMessage, "'length':0"); + + var guid = Guid.NewGuid().ToString(); + Debug.LogError($"LogError(GUID)={guid}"); + + // Wait for screenshot capture to complete + yield return null; + + currentMessage++; // The error event + + t.ExpectMessage(currentMessage, "'type':'event'"); + t.ExpectMessage(currentMessage, $"LogError(GUID)={guid}"); + // Contexts + t.ExpectMessage(currentMessage, "'type':'app',"); + t.ExpectMessage(currentMessage, "'type':'device',"); + t.ExpectMessage(currentMessage, "'type':'gpu',"); + t.ExpectMessage(currentMessage, "'type':'os',"); + t.ExpectMessage(currentMessage, "'type':'runtime',"); + t.ExpectMessage(currentMessage, "'type':'unity',"); + // User + t.ExpectMessage(currentMessage, "'user':{'id':'"); // non-null automatic ID + t.ExpectMessageNot(currentMessage, "'length':0"); + + currentMessage++; // The screenshot envelope + + t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); + t.ExpectMessageNot(currentMessage, "'length':0"); + + SentrySdk.CaptureMessage($"CaptureMessage(GUID)={guid}"); + + // Wait for screenshot capture to complete + yield return null; + + currentMessage++; // The message event + + t.ExpectMessage(currentMessage, "'type':'event'"); + t.ExpectMessage(currentMessage, $"CaptureMessage(GUID)={guid}"); + t.ExpectMessageNot(currentMessage, "'length':0"); + + currentMessage++; // The screenshot envelope + + t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); + t.ExpectMessageNot(currentMessage, "'length':0"); + + var ex = new Exception("Exception & context test"); + AddContext(); + SentrySdk.CaptureException(ex); + + // Wait for screenshot capture to complete + yield return null; + + currentMessage++; // The exception event + + t.ExpectMessage(currentMessage, "'type':'event'"); + t.ExpectMessage(currentMessage, "'message':'crumb','type':'error','data':{'foo':'bar'},'category':'bread','level':'critical'}"); + t.ExpectMessage(currentMessage, "'message':'scope-crumb'}"); + t.ExpectMessage(currentMessage, "'extra':{'extra-key':42}"); + t.ExpectMessage(currentMessage, "'tag-key':'tag-value'"); + t.ExpectMessage(currentMessage, "'user':{'id':'user-id','username':'username','email':'email@example.com','ip_address':'::1','other':{'role':'admin'}}"); + t.ExpectMessageNot(currentMessage, "'length':0"); + + currentMessage++; // The screenshot envelope + + t.ExpectMessage(currentMessage, "'filename':'screenshot.jpg','attachment_type':'event.attachment'"); + t.ExpectMessageNot(currentMessage, "'length':0"); + + Debug.Log("Finished checking messages."); + + t.Pass(); } public static void CrashTest() @@ -259,10 +272,7 @@ protected override Task SendAsync(HttpRequestMessage reques private void Receive(HttpRequestMessage message) { var msgText = message.Content.ReadAsStringAsync().Result; - // Setting "Sentry" as tag to prevent the UnityLogHandlerIntegration from capturing this message and - // adding it as a breadcrumb, which in turn multiplies it on following (intercepted) HTTP requests... - // Note: remove the prefix once setting breadcrumb log level is possible - https://github.com/getsentry/sentry-unity/issues/60 - Debug.unityLogger.Log(LogType.Log, "Sentry", $"{_name} TEST: Intercepted HTTP Request #{_requests.Count} = {msgText}"); + Debug.Log($"{_name} TEST: Intercepted HTTP Request #{_requests.Count} = {msgText}"); _requests.Enqueue(msgText); _requestReceived.Set(); } @@ -306,7 +316,8 @@ public void Expect(string message, bool result) Debug.Log($"{_name} TEST | {_testNumber}. {message}: {(result ? "PASS" : "FAIL")}"); if (!result) { - Debug.Log($"{_name} TEST: FAIL - quitting due to a failed test case #{_testNumber}: '{message}'"); + // run-smoke-test.ps1 expects this as failure confirmation + Debug.Log($"{_name} TEST: FAIL"); Exit(_testNumber); } } diff --git a/test/Sentry.Unity.Tests/ScreenshotEventProcessorTests.cs b/test/Sentry.Unity.Tests/ScreenshotEventProcessorTests.cs index 36252b96e..83e7e5e72 100644 --- a/test/Sentry.Unity.Tests/ScreenshotEventProcessorTests.cs +++ b/test/Sentry.Unity.Tests/ScreenshotEventProcessorTests.cs @@ -1,93 +1,142 @@ -using System.Threading; +using System; +using System.Collections; using NUnit.Framework; using Sentry.Unity.Tests.Stubs; +using UnityEngine; +using UnityEngine.TestTools; namespace Sentry.Unity.Tests; public class ScreenshotEventProcessorTests { - private class Fixture + private class TestScreenshotEventProcessor : ScreenshotEventProcessor { - public SentryUnityOptions Options = new() { AttachScreenshot = true }; - public TestApplication TestApplication = new(); + public Func CaptureScreenshotFunc { get; set; } + public Action CaptureAttachmentAction { get; set; } + public Func WaitForEndOfFrameFunc { get; set; } - public ScreenshotEventProcessor GetSut() => new(Options, TestApplication); - } + public TestScreenshotEventProcessor(SentryUnityOptions options, ISentryMonoBehaviour sentryMonoBehaviour) + : base(options, sentryMonoBehaviour) + { + CaptureScreenshotFunc = _ => [0xFF, 0xD8, 0xFF]; + CaptureAttachmentAction = (_, _) => { }; + WaitForEndOfFrameFunc = () => new YieldInstruction(); + } - private Fixture _fixture = null!; + internal override byte[] CaptureScreenshot(SentryUnityOptions options) + => CaptureScreenshotFunc.Invoke(options); - [SetUp] - public void SetUp() => _fixture = new Fixture(); + internal override void CaptureAttachment(SentryId eventId, SentryAttachment attachment) + => CaptureAttachmentAction(eventId, attachment); - [TearDown] - public void TearDown() - { - if (SentrySdk.IsEnabled) - { - SentrySdk.Close(); - } + internal override YieldInstruction WaitForEndOfFrame() + => WaitForEndOfFrameFunc!.Invoke(); } - [Test] - public void Process_IsMainThread_AddsScreenshotToHint() + public void Process_FirstCallInAFrame_StartsCoroutine() { - _fixture.TestApplication.IsEditor = false; - var sut = _fixture.GetSut(); - var sentryEvent = new SentryEvent(); - var hint = new SentryHint(); + var sentryMonoBehaviour = GetTestMonoBehaviour(); + var screenshotProcessor = new TestScreenshotEventProcessor(new SentryUnityOptions(), sentryMonoBehaviour); - sut.Process(sentryEvent, hint); + screenshotProcessor.Process(new SentryEvent()); - Assert.AreEqual(1, hint.Attachments.Count); + Assert.IsTrue(sentryMonoBehaviour.StartCoroutineCalled); } - [Test] - public void Process_IsNonMainThread_DoesNotAddScreenshotToHint() + [UnityTest] + public IEnumerator Process_ExecutesCoroutine_CapturesScreenshotAndCapturesAttachment() { - var sut = _fixture.GetSut(); - var sentryEvent = new SentryEvent(); - var hint = new SentryHint(); + var sentryMonoBehaviour = GetTestMonoBehaviour(); + var screenshotProcessor = new TestScreenshotEventProcessor(new SentryUnityOptions(), sentryMonoBehaviour); - new Thread(() => + var capturedEventId = SentryId.Empty; + SentryAttachment? capturedAttachment = null; + screenshotProcessor.CaptureAttachmentAction = (eventId, attachment) => { - Thread.CurrentThread.IsBackground = true; - var stream = sut.Process(sentryEvent, hint); + capturedEventId = eventId; + capturedAttachment = attachment; + }; + + var eventId = SentryId.Create(); + var sentryEvent = new SentryEvent(eventId: eventId); + + screenshotProcessor.Process(sentryEvent); - Assert.AreEqual(0, hint.Attachments.Count); - }).Start(); + // Wait for the coroutine to complete - need to wait for processing + yield return null; + yield return null; + + Assert.IsTrue(sentryMonoBehaviour.StartCoroutineCalled); + Assert.AreEqual(eventId, capturedEventId); + Assert.NotNull(capturedAttachment); // Sanity check + Assert.AreEqual("screenshot.jpg", capturedAttachment!.FileName); + Assert.AreEqual("image/jpeg", capturedAttachment.ContentType); + Assert.AreEqual(AttachmentType.Default, capturedAttachment.Type); } - [Test] - [TestCase(true)] - [TestCase(false)] - public void Process_BeforeCaptureScreenshotCallbackProvided_RespectsScreenshotCaptureDecision(bool captureScreenshot) + [UnityTest] + public IEnumerator Process_CalledMultipleTimesQuickly_OnlyExecutesScreenshotCaptureOnce() { - _fixture.TestApplication.IsEditor = false; - _fixture.Options.SetBeforeCaptureScreenshot(() => captureScreenshot); - var sut = _fixture.GetSut(); - var sentryEvent = new SentryEvent(); - var hint = new SentryHint(); + var sentryMonoBehaviour = GetTestMonoBehaviour(); + var screenshotProcessor = new TestScreenshotEventProcessor(new SentryUnityOptions(), sentryMonoBehaviour); + + var screenshotCaptureCallCount = 0; + screenshotProcessor.CaptureScreenshotFunc = _ => + { + screenshotCaptureCallCount++; + return [0]; + }; + + var attachmentCaptureCallCount = 0; + screenshotProcessor.CaptureAttachmentAction = (_, _) => + { + attachmentCaptureCallCount++; + }; + + // Process multiple events quickly (before any coroutine can complete) + screenshotProcessor.Process(new SentryEvent()); + screenshotProcessor.Process(new SentryEvent()); + screenshotProcessor.Process(new SentryEvent()); - sut.Process(sentryEvent, hint); + // Wait for the coroutine to complete - need to wait for processing + yield return null; + yield return null; - Assert.AreEqual(captureScreenshot ? 1 : 0, hint.Attachments.Count); + Assert.AreEqual(1, screenshotCaptureCallCount); + Assert.AreEqual(1, attachmentCaptureCallCount); } - [Test] - [TestCase(true, 0)] - [TestCase(false, 1)] - public void Process_InEditorEnvironment_DoesNotCaptureScreenshot(bool isEditor, int expectedAttachmentCount) + [UnityTest] + public IEnumerator Process_EmptyScreenshotData_SkipsAttachmentCapture() { - // Arrange - _fixture.TestApplication.IsEditor = isEditor; - var sut = _fixture.GetSut(); - var sentryEvent = new SentryEvent(); - var hint = new SentryHint(); + var sentryMonoBehaviour = GetTestMonoBehaviour(); + var screenshotProcessor = new TestScreenshotEventProcessor(new SentryUnityOptions(), sentryMonoBehaviour); + + screenshotProcessor.CaptureScreenshotFunc = _ => []; - // Act - sut.Process(sentryEvent, hint); + var attachmentCaptureCallCount = 0; + screenshotProcessor.CaptureAttachmentAction = (_, _) => + { + attachmentCaptureCallCount++; + }; - // Assert - Assert.AreEqual(expectedAttachmentCount, hint.Attachments.Count); + var eventId = SentryId.Create(); + var sentryEvent = new SentryEvent(eventId: eventId); + + screenshotProcessor.Process(sentryEvent); + + // Wait for the coroutine to complete - need to wait for processing + yield return null; + yield return null; + + Assert.IsTrue(sentryMonoBehaviour.StartCoroutineCalled); + Assert.AreEqual(0, attachmentCaptureCallCount); + } + + private static TestSentryMonoBehaviour GetTestMonoBehaviour() + { + var gameObject = new GameObject("ScreenshotProcessorTest"); + var behaviour = gameObject.AddComponent(); + return behaviour; } } diff --git a/test/Sentry.Unity.Tests/SentryMonoBehaviourTests.cs b/test/Sentry.Unity.Tests/SentryMonoBehaviourTests.cs index 268bb2fcb..8b7e6ed45 100644 --- a/test/Sentry.Unity.Tests/SentryMonoBehaviourTests.cs +++ b/test/Sentry.Unity.Tests/SentryMonoBehaviourTests.cs @@ -1,6 +1,8 @@ +using System.Collections; using NUnit.Framework; using Sentry.Unity.Tests.Stubs; using UnityEngine; +using UnityEngine.TestTools; namespace Sentry.Unity.Tests; @@ -78,4 +80,5 @@ public void UpdatePauseStatus_ResumedTwice_ApplicationResumingInvokedOnlyOnce() Assert.AreEqual(1, counter); } + } diff --git a/test/Sentry.Unity.Tests/Stubs/SentryTestMonoBehaviour.cs b/test/Sentry.Unity.Tests/Stubs/SentryTestMonoBehaviour.cs new file mode 100644 index 000000000..12ffb5add --- /dev/null +++ b/test/Sentry.Unity.Tests/Stubs/SentryTestMonoBehaviour.cs @@ -0,0 +1,19 @@ +using System; +using System.Collections; +using UnityEngine; + +namespace Sentry.Unity.Tests.Stubs; + +internal class TestSentryMonoBehaviour : MonoBehaviour, ISentryMonoBehaviour +{ + public event System.Action? ApplicationResuming; + public void ResumeApplication() => ApplicationResuming?.Invoke(); + + public bool StartCoroutineCalled { get; private set; } + + public new Coroutine StartCoroutine(IEnumerator routine) + { + StartCoroutineCalled = true; + return base.StartCoroutine(routine); + } +} diff --git a/test/Sentry.Unity.Tests/TraceGenerationIntegrationTests.cs b/test/Sentry.Unity.Tests/TraceGenerationIntegrationTests.cs index 91189d44e..470918e21 100644 --- a/test/Sentry.Unity.Tests/TraceGenerationIntegrationTests.cs +++ b/test/Sentry.Unity.Tests/TraceGenerationIntegrationTests.cs @@ -93,11 +93,4 @@ public void TraceGeneration_OnActiveSceneChange_GeneratesNewTrace() Assert.AreNotEqual(initialPropagationContext, scope.PropagationContext); } - - internal class TestSentryMonoBehaviour : ISentryMonoBehaviour - { - public event Action? ApplicationResuming; - - public void ResumeApplication() => ApplicationResuming?.Invoke(); - } }