Sunday project: update xUnit conditional test execution

[ajcvickers]

The main changes here are:

• Fix attributes that were pointing at non-existent classes
• Move conditional check execution to when the tests are running, rather than as part of discovery. This is for two reasons:
  • Code execution during discovery to, for example, attempt to connect to a database is not ideal. Discovery runs in the background and hence discovery code shouldn't do expensive or fragile things.
  • Test runners are free to discover tests without those tests having been built. For example, some test runners do discovery based on an AST. The xUnit metadata model handles this, but only if code doesn't attempt to use actual types when it shouldn't.
• Be as close to the way xUnit natively works. In other words, remove as much test execution logic as possible and extend only when really necessary.

Assembly level condition attributes are problematic for this, since they should apply to all tests in the assembly. That means that all tests need to be ConditionalFact or ConditionalTheory. This allows every test to determine if it should run, even if it's only disabled at the assembly level.

Tested with:

• Command-line runner
• VS runner
• Another one

[AndriySvyryd]

        = "Data Source=(localdb)\\MSSQLLocalDB;Database=master;Integrated Security=True;Connect Timeout=30;ConnectRetryCount=0";

So you want the tests to potentially fail more?

---

Refers to: test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestEnvironment.cs:28 in 399e618. [](commit_id = 399e618, deletion_comment = False)

[smitpatel]

[AndriySvyryd]

    public static bool TrySkip(this XunitTestCase testCase, IMessageBus messageBus)

Couldn't this be just a static method?

---

Refers to: test/EFCore.Specification.Tests/TestUtilities/Xunit/XunitTestCaseExtensions.cs:24 in 399e618. [](commit_id = 399e618, deletion_comment = False)

[AndriySvyryd]

    private CultureInfo _originalUICulture;

💔

---

Refers to: test/EFCore.Specification.Tests/TestUtilities/Xunit/UseCultureAttribute.cs:15 in 399e618. [](commit_id = 399e618, deletion_comment = True)

[ajcvickers]

Re: ;ConnectRetryCount=0. I made the change for two reasons:

• The checked in default in the config file has this, and I figured it should be the same even if the config file is not used.
• It prevents the 10 seconds delay when checking for existence on any database that does not exist. If it makes the tests more flaky, then we need to figure out how to make it more reliable without getting that 10 seconds hit. This should be part of the on-going discussion with SQL Server.

[ajcvickers]

@AndriySvyryd It could be a static method--isn't that true for any extension method? What's the reason you're suggesting this?

[AndriySvyryd]

It seems to only be useful for these implementations so we can avoid having it popup in any referencing projects.

[ajcvickers]

@AndriySvyryd That's reasonable. Will do.