-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Fix to #15994 - Query: investigate suspicious SQL generated by query tests #18803
Conversation
{ | ||
using (var ctx = CreateContext()) | ||
{ | ||
var query = ctx.Entities1.OrderBy(e => e.Id).Select(e => e.NullableStringA.IndexOf("oo")).ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use AssertQuery syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have "real" assert infra for null semantics tests. Will add it at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't MayBe pattern for client solve the purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the AssertQuery we use in this suite is different than what we use everywhere else - it doesnt support custom projections (always assumes we return IQueryable<NullSemanticsEntityBase>
) This was added because we need to pass useRelationalNulls flag to dbcontext, which the normal assert query doesnt provide. However, vast majority of tests uses the c# null semantics so we can just switch to the infra we already have, and places where we would pass useRelationalNulls: true, we can test using CreateContext directly. But I would prefer to do this in a different PR
test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs
Show resolved
Hide resolved
test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Where.cs
Outdated
Show resolved
Hide resolved
@smitpatel feedback addressed, squirrel por favor |
…tests Many tests are not producing expected SQL, tracked remaining ones with relevant issues. Resolves #15994
Many tests are not producing expected SQL, tracked remaining ones with relevant issues.
Resolves #15994