Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make skia runtime tests green locally on Debug #18659

Merged
8 changes: 3 additions & 5 deletions src/SamplesApp/SamplesApp.Shared/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,8 @@ override void OnLaunched(LaunchActivatedEventArgs e)
}

var sw = Stopwatch.StartNew();
#if DEBUG
if (System.Diagnostics.Debugger.IsAttached)
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
{
// this.DebugSettings.EnableFrameRateCounter = true;
}
#if WINAPPSDK && DEBUG
// this.DebugSettings.EnableFrameRateCounter = true;
#endif
AssertInitialWindowSize();

Expand Down Expand Up @@ -549,6 +546,7 @@ static void ConfigureFeatureFlags()
#if HAS_UNO
Uno.UI.FeatureConfiguration.TextBox.UseOverlayOnSkia = false;
Uno.UI.FeatureConfiguration.ToolTip.UseToolTips = true;
Uno.UI.FeatureConfiguration.DependencyProperty.ValidatePropertyOwnerOnReadWrite = true;

Uno.UI.FeatureConfiguration.Font.DefaultTextFontFamily = "ms-appx:///Uno.Fonts.OpenSans/Fonts/OpenSans.ttf";
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ await TestServices.WindowHelper.RootElementDispatcher.RunAsync(() =>
}
else if (test.ExpectedException is null || !test.ExpectedException.IsInstanceOfType(e))
{
if (_currentRun.CurrentRepeatCount < config.Attempts - 1 && !Debugger.IsAttached)
if (_currentRun.CurrentRepeatCount < config.Attempts - 1)
{
_currentRun.CurrentRepeatCount++;
canRetry = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ internal void VerifyNoSelectedDatesChanged()
{
// we expect no event here, so below statement will timeout and throw WEX.Common.Exception.
TestServices.VERIFY_THROWS_WINRT<Exception>(
async () => await m_selectedDatesChangedEvent.WaitFor(TimeSpan.FromMilliseconds(5000), true /* enforceUnderDebugger */),
async () => await m_selectedDatesChangedEvent.WaitFor(TimeSpan.FromMilliseconds(5000)),
"SelectedDatesChanged event should not raise!");
TestServices.VERIFY_IS_FALSE(m_selectedDatesChangedEvent.HasFired());
m_selectedDatesChangedRegistration.Detach();
Expand Down
10 changes: 0 additions & 10 deletions src/Uno.UI.RuntimeTests/IntegrationTests/dxaml/Event.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,5 @@ public Task WaitFor(TimeSpan timeout, CancellationToken ct = default)
{
return WaitForDefault((int)timeout.TotalMilliseconds, ct);
}

internal Task WaitFor(TimeSpan timeout, bool enforceUnderDebugger, CancellationToken ct = default)
{
if (!enforceUnderDebugger && Debugger.IsAttached)
{
return Task.CompletedTask;
}

return WaitForDefault((int)timeout.TotalMilliseconds, ct);
}
}
}
37 changes: 15 additions & 22 deletions src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Microsoft.UI.Xaml;
using UIExecutor = MUXControlsTestApp.Utilities.RunOnUIThread;
using MUXControlsTestApp.Utilities;
using Uno.UI;

namespace Microsoft/* UWP don't rename */.UI.Xaml.Tests.Common
{
Expand Down Expand Up @@ -92,11 +93,11 @@
#if !__WASM__
if (this.options.HasFlag(EventTesterOptions.CaptureWindowBefore))
{
this.CaptureWindowAsync("Before").Wait(this.Timeout);
this.CaptureWindowAsync("Before").Wait(this.DefaultTimeout);

Check failure on line 96 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L96

Replace this use of 'Task.Wait' with 'await'.
}
if (this.options.HasFlag(EventTesterOptions.CaptureScreenBefore))
{
this.CaptureScreenAsync("Before").Wait(this.Timeout);
this.CaptureScreenAsync("Before").Wait(this.DefaultTimeout);

Check failure on line 100 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L100

Replace this use of 'Task.Wait' with 'await'.
}
#endif
}
Expand Down Expand Up @@ -153,22 +154,14 @@
return new RoutedEventTester<TEventArgs>(sender, eventName, action);
}

public TimeSpan DefaultTimeout = EventTesterConfig.Timeout;

private TimeSpan Timeout
{
get
{
if (Debugger.IsAttached)
{
return TimeSpan.FromMilliseconds(-1); // Wait indefinitely if debugger is attached.
}
else
{
return this.DefaultTimeout;
}
}
}
public TimeSpan DefaultTimeout =

Check warning on line 157 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L157

Make 'DefaultTimeout' private.

Check notice on line 157 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L157

Make this field 'private' and encapsulate it in a 'public' property.
#if HAS_UNO
FeatureConfiguration.DebugOptions.WaitIndefinitelyInEventTester
#else
Debugger.IsAttached
#endif
? TimeSpan.FromMilliseconds(-1)
: EventTesterConfig.Timeout;

private TSender Sender
{
Expand Down Expand Up @@ -258,7 +251,7 @@

public async Task<bool> Wait()
{
return await this.Wait(this.Timeout);
return await this.Wait(this.DefaultTimeout);
}

public async Task<bool> Wait(TimeSpan timeout)
Expand Down Expand Up @@ -332,7 +325,7 @@

public async Task<bool> WaitAsync()
{
return await this.WaitAsync(this.Timeout);
return await this.WaitAsync(this.DefaultTimeout);
}

public async Task<bool> WaitAsync(TimeSpan timeout)
Expand Down Expand Up @@ -441,11 +434,11 @@
#if !__WASM__
if (this.options.HasFlag(EventTesterOptions.CaptureWindowAfter))
{
this.CaptureWindowAsync("After").Wait(this.Timeout);
this.CaptureWindowAsync("After").Wait(this.DefaultTimeout);

Check failure on line 437 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L437

Replace this use of 'Task.Wait' with 'await'.
}
if (this.options.HasFlag(EventTesterOptions.CaptureScreenAfter))
{
this.CaptureScreenAsync("After").Wait(this.Timeout);
this.CaptureScreenAsync("After").Wait(this.DefaultTimeout);

Check failure on line 441 in src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI.RuntimeTests/MUX/Helpers/EventTester.cs#L441

Replace this use of 'Task.Wait' with 'await'.
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI.RuntimeTests/MUX/Utilities/TestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ namespace MUXControlsTestApp.Utilities
{
public static class TestUtilities
{
public static int DefaultWaitMs = Debugger.IsAttached ? 120000 : 5000;
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
public static int DefaultWaitMs = 5000;

public static async Task SetAsVisualTreeRoot(FrameworkElement element)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class Given_UnitTest
{
static int When_UnhandledException_Count;

// This tests that automatic retry is working.
[TestMethod]
public void When_UnhandledException()
{
Expand All @@ -25,6 +26,7 @@ public class Given_UnitTest_Initialize
{
static int Initialize_Count;

// This tests that automatic retry is working.
[TestInitialize]
public void Initialize()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,8 @@ public async Task When_Exif_Rotated_MsAppx_Unequal_Dimensions()
await WindowHelper.WaitFor(() => imageOpened);
var screenshot = await TakeScreenshot(image);
ImageAssert.HasColorAt(screenshot, 5, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
ImageAssert.HasColorAt(screenshot, screenshot.Width / 2 - 10, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
ImageAssert.HasColorAt(screenshot, screenshot.Width / 2 + 10, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);
ImageAssert.HasColorAt(screenshot, screenshot.Width / 2.2f, screenshot.Height / 2, Color.FromArgb(0xFF, 0xED, 0x1B, 0x24), tolerance: 5);
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
ImageAssert.HasColorAt(screenshot, screenshot.Width / 1.8f, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);
ImageAssert.HasColorAt(screenshot, screenshot.Width - 5, screenshot.Height / 2, Color.FromArgb(0xFF, 0x23, 0xB1, 0x4D), tolerance: 5);

}
Expand Down
23 changes: 23 additions & 0 deletions src/Uno.UI/FeatureConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,29 @@ public static class DependencyProperty
/// of having an undefined behavior and/or race conditions.
/// </summary>
public static bool DisableThreadingCheck { get; set; }

/// <summary>
/// Enables checks that make sure that <see cref="DependencyObjectStore.GetValue" /> and
/// <see cref="DependencyObjectStore.SetValue" /> are only called on the owner of the property being
/// set/got.
/// </summary>
public static bool ValidatePropertyOwnerOnReadWrite { get; set; } =
#if DEBUG
true;
#else
global::System.Diagnostics.Debugger.IsAttached;
#endif
}

/// <summary>
/// This is for internal use to facilitate turning on/off certain logic that makes it easier/harder
/// to debug.
/// </summary>
internal static class DebugOptions
{
public static bool PreventKeyboardStateTrackerFromResettingOnWindowActivationChange { get; set; }

public static bool WaitIndefinitelyInEventTester { get; set; }
}
}
}
3 changes: 2 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/AppBar/AppBar.Partial.cs
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,8 @@ private void OnIsOpenChanged(bool isOpen)
{
// If the AppBar is not live, then wait until it's loaded before
// responding to changes to opened state and firing our Opening/Opened events.
if (!IsInLiveTree)
// Uno Specific: using IsLoaded instead of IsInLiveTree, which makes more sense because OnOpening (called below) -> SetupOverlayState expects OnApplyTemplate to have already been called
if (!IsLoaded)
{
ramezgerges marked this conversation as resolved.
Show resolved Hide resolved
return;
}
Expand Down
4 changes: 1 addition & 3 deletions src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ public static class TraceProvider
/// </summary>
private SpecializedResourceDictionary.ResourceKey? _themeLastUsed;

private static readonly bool _validatePropertyOwner = Debugger.IsAttached;

#if UNO_HAS_ENHANCED_LIFECYCLE
internal bool IsDisposed => _isDisposed;
#endif
Expand Down Expand Up @@ -802,7 +800,7 @@ private void WritePropertyEventTrace(int eventId, DependencyProperty property, D
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void ValidatePropertyOwner(DependencyProperty property)
{
if (_validatePropertyOwner)
if (FeatureConfiguration.DependencyProperty.ValidatePropertyOwnerOnReadWrite)
{
var isFrameworkElement = _originalObjectType.Is(typeof(FrameworkElement));
var isMixinFrameworkElement = _originalObjectRef.Target is IFrameworkElement && !isFrameworkElement;
Expand Down
2 changes: 2 additions & 0 deletions src/Uno.UI/UI/Xaml/UIElementCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public UIElement this[int index]

public void Add(UIElement item)
{
#if !__CROSSRUNTIME__ // SetParent is already called in AddCore and calling it here messes up the check inside AddCore for a preexisting parent. VerifyNavigationViewItemToolTipPaneDisplayMode (in DEBUG) fails otherwise because Enter is called multiple times on the same element
item.SetParent(_owner);
#endif

AddCore(item);
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,10 @@ private void OnActivationStateChanged(CoreWindowActivationState state)
CoreWindow?.OnActivated(coreWindowActivatedEventArgs);
Activated?.Invoke(Window, activatedEventArgs);
SystemThemeHelper.RefreshSystemTheme();
KeyboardStateTracker.Reset();
if (!FeatureConfiguration.DebugOptions.PreventKeyboardStateTrackerFromResettingOnWindowActivationChange)
{
KeyboardStateTracker.Reset();
}
}

public bool Close()
Expand Down
8 changes: 7 additions & 1 deletion src/Uno.UWP/Buffers/DefaultArrayPoolBucket.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ private sealed class Bucket
/// </summary>
internal Bucket(int bufferLength, int numberOfBuffers, int poolId)
{
_lock = new SpinLock(Debugger.IsAttached); // only enable thread tracking if debugger is attached; it adds non-trivial overheads to Enter/Exit
_lock = new SpinLock(
#if DEBUG // thread tracking adds non-trivial overheads to Enter/Exit
true
#else
global::System.Diagnostics.Debugger.IsAttached
#endif
);
_buffers = new T[numberOfBuffers][];
_bufferLength = bufferLength;
_poolId = poolId;
Expand Down
10 changes: 1 addition & 9 deletions src/Uno.UWP/UI/Core/Internal/KeyboardStateTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,7 @@ private static void SetStateOnNonSideKeys(VirtualKey key)
}
}

internal static void Reset()
{
// Clearing state when debugger is attached would
// make it hard to debug key events.
if (!System.Diagnostics.Debugger.IsAttached)
{
_keyStates.Clear();
}
}
internal static void Reset() => _keyStates.Clear();
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved

#if __WASM__
#pragma warning disable IDE0051 // Remove unused private members
Expand Down
Loading