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

Refactoring query tests so that they are run both synchronously and asynchronously. #12500

Closed
wants to merge 1 commit into from

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Jun 28, 2018

Tests are now Theories, not Facts and take a single parameter bool isAsync. Some tests got converted from sync to async so this will cause compilation errors for the providers who override those tests (e.g. to verify SQL)

// public abstract class AsyncQueryTestBase<TFixture> : IClassFixture<TFixture>
// where TFixture : class, IQueryFixtureBase, new()
// {
// protected AsyncQueryTestBase(TFixture fixture) => Fixture = fixture;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// [](start = 0, length = 3)

Remove commented out code

@AndriySvyryd
Copy link
Member

    //public virtual Task Client_where_GroupBy_Group_ordering_works()

Remove


Refers to: src/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs:603 in 89f2766. [](commit_id = 89f2766, deletion_comment = False)

…synchronously.

Tests are now Theories, not Facts and take a single parameter bool isAsync. Some tests got converted from sync to async so this will cause compilation errors for the providers who override those tests (e.g. to verify SQL)

#region AssertAnyWithPredicate

protected virtual Task AssertAnyWithPredicate<TItem1, TPredicate>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not stick to overloads mirroring LINQ? I.e AssertAny instead of AssertAnyWithPredicate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will change

@AndriySvyryd
Copy link
Member

    public abstract Task AssertAnyWithPredicate<TItem1, TPredicate>(

Same here


Refers to: src/EFCore.Specification.Tests/TestUtilities/QueryAsserterBase.cs:221 in 89f2766. [](commit_id = 89f2766, deletion_comment = False)

Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@natemcmaster natemcmaster changed the base branch from dev to master July 2, 2018 17:58
@AndriySvyryd
Copy link
Member

This is already merged

@smitpatel smitpatel deleted the async_tests branch July 17, 2018 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants