From 1a78b52a40c5954174bb823ec13725e385d73aab Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 16 Jun 2020 21:04:09 -0700 Subject: [PATCH] Add capability to verify order in ordered filtered include queries ExpectedFilteredInclude now can be marked to verify order - by default we don't do it. --- .../Query/ComplexNavigationsQueryTestBase.cs | 67 ++++++++++++------- .../Query/NorthwindIncludeQueryTestBase.cs | 3 +- .../TestUtilities/ExpectedFilteredInclude.cs | 6 +- .../TestUtilities/QueryAsserter.cs | 20 ++++-- 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs index 1389e7be711..04b18136a3c 100644 --- a/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs @@ -5006,7 +5006,8 @@ public virtual Task Filtered_include_OrderBy(bool async) elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.OrderBy(x => x.Name)))); + includeFilter: x => x.OrderBy(x => x.Name), + assertOrder: true))); } [ConditionalTheory] @@ -5021,7 +5022,8 @@ public virtual Task Filtered_ThenInclude_OrderBy(bool async) new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToMany_Optional1", - includeFilter: x => x.OrderBy(x => x.Name)))); + includeFilter: x => x.OrderBy(x => x.Name), + assertOrder: true))); } [ConditionalTheory] @@ -5036,11 +5038,13 @@ public virtual Task Filtered_include_ThenInclude_OrderBy(bool async) elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.OrderBy(x => x.Name)), + includeFilter: x => x.OrderBy(x => x.Name), + assertOrder: true), new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToMany_Optional1", - includeFilter: x => x.OrderByDescending(x => x.Name)))); + includeFilter: x => x.OrderByDescending(x => x.Name), + assertOrder: true))); } [ConditionalTheory] @@ -5053,7 +5057,8 @@ public virtual Task Filtered_include_basic_OrderBy_Take(bool async) elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.OrderBy(x => x.Name).Take(3)))); + includeFilter: x => x.OrderBy(x => x.Name).Take(3), + assertOrder: true))); } [ConditionalTheory] @@ -5066,7 +5071,8 @@ public virtual Task Filtered_include_basic_OrderBy_Skip(bool async) elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.OrderBy(x => x.Name).Skip(1)))); + includeFilter: x => x.OrderBy(x => x.Name).Skip(1), + assertOrder: true))); } [ConditionalTheory] @@ -5079,7 +5085,8 @@ public virtual Task Filtered_include_basic_OrderBy_Skip_Take(bool async) elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3)))); + includeFilter: x => x.OrderBy(x => x.Name).Skip(1).Take(3), + assertOrder: true))); } [ConditionalFact] @@ -5112,7 +5119,8 @@ public virtual Task Filtered_include_on_ThenInclude(bool async) new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToOne_Optional_FK1", - x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3)))); + x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3), + assertOrder: true))); } [ConditionalTheory] @@ -5128,10 +5136,11 @@ public virtual Task Filtered_include_after_reference_navigation(bool async) new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToOne_Optional_FK1", - x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3)))); + x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Skip(1).Take(3), + assertOrder: true))); } - [ConditionalTheory] + [ConditionalTheory(Skip = "issue #21338")] [MemberData(nameof(IsAsyncData))] public virtual Task Filtered_include_after_different_filtered_include_same_level(bool async) { @@ -5143,10 +5152,12 @@ public virtual Task Filtered_include_after_different_filtered_include_same_level elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3), + assertOrder: true), new ExpectedFilteredInclude( e => e.OneToMany_Required1, - includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1)))); + includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1), + assertOrder: true))); } [ConditionalTheory] @@ -5161,11 +5172,13 @@ public virtual Task Filtered_include_after_different_filtered_include_different_ elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Name).Take(3), + assertOrder: true), new ExpectedFilteredInclude( e => e.OneToMany_Required2, "OneToMany_Optional1", - includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1)))); + includeFilter: x => x.Where(x => x.Name != "Bar").OrderByDescending(x => x.Name).Skip(1), + assertOrder: true))); } [ConditionalTheory] @@ -5192,7 +5205,7 @@ public virtual async Task Filtered_include_different_filter_set_on_same_navigati .Include(l1 => l1.OneToMany_Optional1.Where(x => x.Name != "Bar")).ThenInclude(l2 => l2.OneToOne_Required_FK2)))).Message; } - [ConditionalTheory] + [ConditionalTheory(Skip = "issue #21338")] [MemberData(nameof(IsAsyncData))] public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice(bool async) { @@ -5204,7 +5217,8 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice(bo elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderByDescending(x => x.Id).Take(2)))); + includeFilter: x => x.Where(x => x.Name != "Foo").OrderByDescending(x => x.Id).Take(2), + assertOrder: true))); } [ConditionalTheory] @@ -5219,7 +5233,8 @@ public virtual Task Filtered_include_same_filter_set_on_same_navigation_twice_fo elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2), + assertOrder: true), new ExpectedInclude(e => e.OneToMany_Optional2), new ExpectedInclude(e => e.OneToOne_Required_FK2))); } @@ -5236,7 +5251,8 @@ public virtual Task Filtered_include_multiple_multi_level_includes_with_first_le elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(2), + assertOrder: true), new ExpectedInclude(e => e.OneToMany_Optional2, "OneToMany_Optional1"), new ExpectedInclude(e => e.OneToOne_Required_FK2, "OneToMany_Optional1"))); } @@ -5253,7 +5269,8 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3)))); + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3), + assertOrder: true))); } [ConditionalTheory] @@ -5268,7 +5285,8 @@ public virtual Task Filtered_include_and_non_filtered_include_on_same_navigation elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3)))); + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(3), + assertOrder: true))); } [ConditionalTheory] @@ -5283,7 +5301,8 @@ public virtual Task Filtered_include_and_non_filtered_include_followed_by_then_i elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude( e => e.OneToMany_Optional1, - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1), + assertOrder: true), new ExpectedInclude(e => e.OneToOne_Optional_PK2, "OneToMany_Optional1"), new ExpectedFilteredInclude( e => e.OneToMany_Optional3, @@ -5305,7 +5324,8 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToMany_Optional1", - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1), + assertOrder: true), new ExpectedInclude(e => e.OneToMany_Optional3, "OneToMany_Optional1.OneToMany_Optional2"), new ExpectedInclude(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2"))); } @@ -5324,7 +5344,8 @@ public virtual Task Filtered_include_complex_three_level_with_middle_having_filt new ExpectedFilteredInclude( e => e.OneToMany_Optional2, "OneToMany_Optional1", - includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1)), + includeFilter: x => x.Where(x => x.Name != "Foo").OrderBy(x => x.Id).Take(1), + assertOrder: true), new ExpectedInclude(e => e.OneToMany_Optional3, "OneToMany_Optional1.OneToMany_Optional2"), new ExpectedInclude(e => e.OneToMany_Required3, "OneToMany_Optional1.OneToMany_Optional2"))); } diff --git a/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs index b90a3a93599..117714aad12 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindIncludeQueryTestBase.cs @@ -1800,7 +1800,8 @@ public virtual Task Filtered_include_with_multiple_ordering(bool async) .Include(c => c.Orders.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate)), elementAsserter: (e, a) => AssertInclude(e, a, new ExpectedFilteredInclude(c => c.Orders, - includeFilter: os => os.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate))), + includeFilter: os => os.OrderBy(o => o.OrderID).Skip(1).OrderByDescending(o => o.OrderDate), + assertOrder: true)), entryCount: 64); } diff --git a/test/EFCore.Specification.Tests/TestUtilities/ExpectedFilteredInclude.cs b/test/EFCore.Specification.Tests/TestUtilities/ExpectedFilteredInclude.cs index 17000f5402f..3c54594513f 100644 --- a/test/EFCore.Specification.Tests/TestUtilities/ExpectedFilteredInclude.cs +++ b/test/EFCore.Specification.Tests/TestUtilities/ExpectedFilteredInclude.cs @@ -11,13 +11,17 @@ public class ExpectedFilteredInclude : ExpectedInclude, IEnumerable> IncludeFilter { get; } + public bool AssertOrder { get; } + public ExpectedFilteredInclude( Expression>> include, string navigationPath = "", - Func, IEnumerable> includeFilter = null) + Func, IEnumerable> includeFilter = null, + bool assertOrder = false) : base(Convert(include), navigationPath) { IncludeFilter = includeFilter; + AssertOrder = assertOrder; } private static Expression> Convert(Expression>> include) diff --git a/test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs b/test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs index fbd609aad57..52660ffe76f 100644 --- a/test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs +++ b/test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs @@ -1493,10 +1493,10 @@ public void AssertInclude(TEntity expected, TEntity actual, IExpectedIn { _includePath.Clear(); - AssertIncludeObject(expected, actual, expectedIncludes); + AssertIncludeObject(expected, actual, expectedIncludes, assertOrder: false); } - private void AssertIncludeObject(object expected, object actual, IEnumerable expectedIncludes) + private void AssertIncludeObject(object expected, object actual, IEnumerable expectedIncludes, bool assertOrder) { if (expected == null && actual == null) @@ -1512,7 +1512,7 @@ private void AssertIncludeObject(object expected, object actual, IEnumerable i.IsConstructedGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>))) { _assertIncludeCollectionMethodInfo.MakeGenericMethod(expectedType.GenericTypeArguments[0]) - .Invoke(this, new[] { expected, actual, expectedIncludes }); + .Invoke(this, new[] { expected, actual, expectedIncludes, assertOrder }); } else { @@ -1534,12 +1534,15 @@ private void AssertIncludeEntity(TElement expected, TElement actual, I } private void AssertIncludeCollection( - IEnumerable expected, IEnumerable actual, IEnumerable expectedIncludes) + IEnumerable expected, + IEnumerable actual, + IEnumerable expectedIncludes, + bool assertOrder) { var expectedList = expected.ToList(); var actualList = actual.ToList(); - if (_entitySorters.TryGetValue(typeof(TElement), out var sorter)) + if (!assertOrder && _entitySorters.TryGetValue(typeof(TElement), out var sorter)) { var actualSorter = (Func)sorter; expectedList = expectedList.OrderBy(actualSorter).ToList(); @@ -1563,6 +1566,7 @@ private void ProcessIncludes(TEntity expected, TEntity actual, IEnumera foreach (var expectedInclude in expectedIncludes.OfType>().Where(i => i.NavigationPath == currentPath)) { var expectedIncludedNavigation = GetIncluded(expected, expectedInclude.IncludeMember); + var assertOrder = false; if (expectedInclude.GetType().BaseType != typeof(object)) { var includedType = expectedInclude.GetType().GetGenericArguments()[1]; @@ -1573,13 +1577,17 @@ private void ProcessIncludes(TEntity expected, TEntity actual, IEnumera null, new object[] { expectedIncludedNavigation, expectedInclude }, CultureInfo.CurrentCulture); + + assertOrder = (bool)expectedInclude.GetType() + .GetProperty(nameof(ExpectedFilteredInclude.AssertOrder)) + .GetValue(expectedInclude); } var actualIncludedNavigation = GetIncluded(actual, expectedInclude.IncludeMember); _includePath.Add(expectedInclude.IncludeMember.Name); - AssertIncludeObject(expectedIncludedNavigation, actualIncludedNavigation, expectedIncludes); + AssertIncludeObject(expectedIncludedNavigation, actualIncludedNavigation, expectedIncludes, assertOrder); _includePath.RemoveAt(_includePath.Count - 1); }