Skip to content

Conversation

@roji
Copy link
Member

@roji roji commented Nov 14, 2019

This splits all our "simple" tests, which are currently in a single class (split across files via partial), into separate classes.

Still TODO: set up the Northwind database exactly once for all suites. xunit supports "shared context" via the concept of test collections, but this also has an impact on parallelism. I've posted a question on stackoverflow, @AndriySvyryd @maumar any ideas on this?

@smitpatel
Copy link
Contributor

  • Based on benchmarking I did a while ago, there is not much need to run them in parallel time-wise though it would be good to have.
  • Given we already have different test classes using NorthwindFixture which are running in parallel, according to @AndriySvyryd infra should already be capable of running them in parallel without anything specific to be done.

@roji roji merged commit 0bb344f into master Nov 15, 2019
@roji roji deleted the ByeByeSimple branch November 15, 2019 05:56
namespace Microsoft.EntityFrameworkCore.Query
{
public abstract partial class SimpleQueryTestBase<TFixture>
public abstract class NorthwindAggregateOperatorsQueryTestBase<TFixture> : QueryTestBase<TFixture>
Copy link
Member

Choose a reason for hiding this comment

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

Why are they prefixed with Northwind? Usually the test classes are named after the feature they are testing.

Copy link
Member

Choose a reason for hiding this comment

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

Adding Query in the name is also redundant in most cases


In reply to: 346674959 [](ancestors = 346674959)

Copy link
Contributor

Choose a reason for hiding this comment

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

GearsOfWarQueryTestbase 🗡 We need to determine a consistent convention to name test classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants