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.

Sign up for GitHub

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

TaskResultConverter returns default value when task not set #4790

Merged
merged 4 commits into from
Oct 21, 2022
Merged

TaskResultConverter returns default value when task not set #4790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e3b4e0
Use new C# new initialization for vars
michael-hawker Oct 20, 2022
b8ec35e
Add new test cases which show issues with current TaskResultConverter.
michael-hawker Oct 20, 2022
1398210
Fixes #4505 - TaskResultConverter properly returns default value in c…
michael-hawker Oct 20, 2022
e3fe539
Remove ValueType support and reflection piece from TaskResultConverter
michael-hawker Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Microsoft.Toolkit.Uwp.UI/Converters/TaskResultConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Converters
/// This is needed because accessing <see cref="Task{TResult}.Result"/> when the task has not
/// completed yet will block the current thread and might cause a deadlock (eg. if the task was
/// scheduled on the same synchronization context where the result is being retrieved from).
/// The methods in this converter will safely return <see langword="default"/> if the input
/// task is still running, or if it has faulted or has been canceled.
/// The methods in this converter will safely return <see langword="null"/> if the input
/// task is not set yet, still running, has faulted, or has been canceled.
/// </summary>
public sealed class TaskResultConverter : IValueConverter
{
Expand All @@ -26,8 +26,13 @@ public object Convert(object value, Type targetType, object parameter, string la
{
return task.GetResultOrDefault();
}
else if (value is null)
{
return null;
}

return DependencyProperty.UnsetValue;
// Otherwise, we'll just pass through whatever value/result was given to us.
return value;
}

/// <inheritdoc/>
Expand Down
81 changes: 51 additions & 30 deletions UnitTests/UnitTests.UWP/Converters/Test_TaskResultConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,97 +17,118 @@ public class Test_TaskResultConverter
{
[TestCategory("Converters")]
[UITestMethod]
[Ignore] // Ignore this value type test. Behavior will return null currently and not default.
public void Test_TaskResultConverter_Instance_Int32()
{
var converter = new TaskResultConverter();
TaskResultConverter converter = new();

TaskCompletionSource<int> tcs = new TaskCompletionSource<int>();
TaskCompletionSource<int> tcs = new();

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(0, (int)converter.Convert(tcs.Task, typeof(int), null, null));

tcs.SetCanceled();

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(0, (int)converter.Convert(tcs.Task, typeof(int), null, null));

tcs = new TaskCompletionSource<int>();

tcs.SetException(new InvalidOperationException("Test"));

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(0, (int)converter.Convert(tcs.Task, typeof(int), null, null));

tcs = new TaskCompletionSource<int>();

tcs.SetResult(42);

Assert.AreEqual(42, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(42, (int)converter.Convert(tcs.Task, typeof(int), null, null));
}

[TestCategory("Converters")]
[UITestMethod]
public void Test_TaskResultConverter_Instance_String()
{
var converter = new TaskResultConverter();
TaskResultConverter converter = new();

TaskCompletionSource<string> tcs = new TaskCompletionSource<string>();
TaskCompletionSource<string> tcs = new();

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(null, (string)converter.Convert(tcs.Task, typeof(string), null, null));

tcs.SetCanceled();

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(null, (string)converter.Convert(tcs.Task, typeof(string), null, null));

tcs = new TaskCompletionSource<string>();
tcs = new();

tcs.SetException(new InvalidOperationException("Test"));

Assert.AreEqual(null, converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual(null, (string)converter.Convert(tcs.Task, typeof(string), null, null));

tcs = new TaskCompletionSource<string>();
tcs = new();

tcs.SetResult("Hello world");

Assert.AreEqual("Hello world", converter.Convert(tcs.Task, null, null, null));
Assert.AreEqual("Hello world", (string)converter.Convert(tcs.Task, typeof(string), null, null));
}

[TestCategory("Converters")]
[UITestMethod]
public void Test_TaskResultConverter_Instance_UnsetValue()
public void Test_TaskResultConverter_Instance_RawValue()
{
var converter = new TaskResultConverter();
TaskResultConverter converter = new();

Assert.AreEqual(DependencyProperty.UnsetValue, converter.Convert(null, null, null, null));
Assert.AreEqual(DependencyProperty.UnsetValue, converter.Convert("Hello world", null, null, null));
Assert.AreEqual(42, converter.Convert(42, null, null, null));

Assert.AreEqual(42, converter.Convert(42, typeof(int), null, null));

Assert.AreEqual("Hello world", converter.Convert("Hello world", null, null, null));

Assert.AreEqual("Hello world", converter.Convert("Hello world", typeof(string), null, null));
}

[TestCategory("Converters")]
[UITestMethod]
public void Test_TaskResultConverter_Instance_Null()
public void Test_TaskResultConverter_Instance_NullObject()
{
var converter = new TaskResultConverter();
TaskResultConverter converter = new();

var cts = new CancellationTokenSource();
Assert.AreEqual(null, converter.Convert(null, null, null, null));

cts.Cancel();
// TODO: Think there may still be a problem for value types in x:Bind expressions, represented by these tests here,
// but was going to be too big a change for 7.1.3, will have to get more feedback and evaluate later.
/*Assert.AreEqual(0, (int)converter.Convert(null, typeof(int), null, null));

Assert.AreEqual(null, converter.Convert(Task.FromCanceled(cts.Token), null, null, null));
Assert.AreEqual(null, converter.Convert(Task.FromException(new Exception()), null, null, null));
Assert.AreEqual(null, converter.Convert(Task.CompletedTask, null, null, null));
Assert.AreEqual(false, (bool)converter.Convert(null, typeof(bool), null, null));*/

TaskCompletionSource<int> tcs1 = new TaskCompletionSource<int>();
Assert.AreEqual(null, converter.Convert(null, typeof(int), null, null));

Assert.AreEqual(null, converter.Convert(tcs1.Task, null, null, null));
Assert.AreEqual(null, converter.Convert(null, typeof(bool), null, null));

TaskCompletionSource<string> tcs2 = new TaskCompletionSource<string>();
Assert.AreEqual(null, (int?)converter.Convert(null, typeof(int?), null, null));

Assert.AreEqual(null, converter.Convert(tcs2.Task, null, null, null));
Assert.AreEqual(null, (string)converter.Convert(null, typeof(string), null, null));
}

[TestCategory("Converters")]
[UITestMethod]
public void Test_TaskResultConverter_Instance_TaskNull()
{
TaskResultConverter converter = new();

CancellationTokenSource cts = new();

cts.Cancel();

Assert.AreEqual(null, converter.Convert(Task.FromCanceled(cts.Token), null, null, null));
Assert.AreEqual(null, converter.Convert(Task.FromException(new Exception()), null, null, null));
Assert.AreEqual(null, converter.Convert(Task.CompletedTask, null, null, null));
}

[TestCategory("Converters")]
[UITestMethod]
[ExpectedException(typeof(NotImplementedException))]
public void Test_TaskResultConverter_Instance_ConvertBack()
{
var converter = new TaskResultConverter();
TaskResultConverter converter = new();

Assert.AreEqual(null, converter.ConvertBack(null, null, null, null));
}
Expand Down