feat: Automatic trace instrumentation for MAUI #5138
2 issues
code-review: Found 2 issues (1 high, 1 medium)
High
Test will fail: Hub.GetSpan() not mocked, causing StartNavigationSpan to return null - `test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs:209-215`
The test OnShellOnNavigated_FinishesNavSpan is missing the _fixture.Hub.GetSpan().Returns(mockTransaction) setup that other similar tests in this file have (see lines 70, 92, 109). Without this mock, StartNavigationSpan checks if (_hub.GetSpan() is not { IsFinished: false } parentSpan) and returns null immediately. This causes the Assert.NotNull(navSpan) assertion to fail, and subsequent assertions on navSpan.Description and navSpan.Received(1).Finish() will also fail.
Also found at:
test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs:128-148
Medium
Test asserts on wrong Scope object, potentially masking bugs - `test/Sentry.Maui.Tests/MauiEventsBinderTests.Button.cs:285`
The test StartUiTransaction_TransactionOnScope_NotBoundToScope creates a new local scope and configures the Hub mock to use it via SubstituteConfigureScope(scope) at line 269. However, the assertion at line 285 checks _fixture.Scope.Transaction instead of the local scope.Transaction. Since SubstituteConfigureScope overwrites the mock to use the new scope, the assertion is checking the original fixture scope that is no longer connected to the Hub's ConfigureScope behavior. This test may pass even if the implementation is broken because it's not checking the correct scope object.
Duration: 5m 44s · Tokens: 5.5M in / 65.6k out · Cost: $7.83 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.01)
Annotations
Check failure on line 215 in test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs
sentry-warden / warden: code-review
Test will fail: Hub.GetSpan() not mocked, causing StartNavigationSpan to return null
The test `OnShellOnNavigated_FinishesNavSpan` is missing the `_fixture.Hub.GetSpan().Returns(mockTransaction)` setup that other similar tests in this file have (see lines 70, 92, 109). Without this mock, `StartNavigationSpan` checks `if (_hub.GetSpan() is not { IsFinished: false } parentSpan)` and returns `null` immediately. This causes the `Assert.NotNull(navSpan)` assertion to fail, and subsequent assertions on `navSpan.Description` and `navSpan.Received(1).Finish()` will also fail.
Check failure on line 148 in test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs
sentry-warden / warden: code-review
[PSK-HNL] Test will fail: Hub.GetSpan() not mocked, causing StartNavigationSpan to return null (additional location)
The test `OnShellOnNavigated_FinishesNavSpan` is missing the `_fixture.Hub.GetSpan().Returns(mockTransaction)` setup that other similar tests in this file have (see lines 70, 92, 109). Without this mock, `StartNavigationSpan` checks `if (_hub.GetSpan() is not { IsFinished: false } parentSpan)` and returns `null` immediately. This causes the `Assert.NotNull(navSpan)` assertion to fail, and subsequent assertions on `navSpan.Description` and `navSpan.Received(1).Finish()` will also fail.
Check warning on line 285 in test/Sentry.Maui.Tests/MauiEventsBinderTests.Button.cs
sentry-warden / warden: code-review
Test asserts on wrong Scope object, potentially masking bugs
The test `StartUiTransaction_TransactionOnScope_NotBoundToScope` creates a new local `scope` and configures the Hub mock to use it via `SubstituteConfigureScope(scope)` at line 269. However, the assertion at line 285 checks `_fixture.Scope.Transaction` instead of the local `scope.Transaction`. Since `SubstituteConfigureScope` overwrites the mock to use the new scope, the assertion is checking the original fixture scope that is no longer connected to the Hub's ConfigureScope behavior. This test may pass even if the implementation is broken because it's not checking the correct scope object.