mysql-net/AdoNetApiTest

Create abstract test suite

Closed this issue · 10 comments

bgrainger commented

See https://github.com/dotnet/corefx/issues/7810#issuecomment-346144759 where @roji wrote:

there will need to be a generic abstract nuget that specific ADO.NET drivers can consume, extending the abstract test suite. Some tests may be overridden (so everything needs to be virtual), some would be ignored/skipped (not relevant for that provider), etc. etc.

A previous comment mentions "EFCore.Specification.Tests"; the latest code path is https://github.com/aspnet/EntityFrameworkCore/tree/dev/src/EFCore.Specification.Tests.

Probably the patterns developed in that test suite would be appropriate to use here.

It looks like it takes a dependency on Xunit. If we wanted to keep the colour-coded matrix, maybe it could be created by parsing Xunit output from running the tests with a particular connector.

bgrainger commented

Established the basic pattern that test classes should follow, and implemented it for MySqlConnector.

Using Xunit brings nice benefits, such as being able to run/debug individual tests in an IDE. Creating the same table from its XML output may be challenging, though.

bgrainger commented

All tests have been ported to the new framework (but a NuGet package has not been published yet).

roji commented

Once this is published as a nuget I'll also set up a project in the Npgsql solution and see how it goes...

bgrainger commented

FWIW, here are the current Npgsql test "failures". I don't yet have a good way to denote "let's agree to disagree"; maybe overriding the method with a Skip reason? (Of course, when integrated into your own test suite you're free to do whatever you want.)

DbCommand

  • Setting CommandText or Connection when there is an active reader; expected to throw InvalidOperationException; Npgsql does not.
    • Only System.Data.SQLite, pre-release Microsoft.Data.Sqlite, and latest MySqlConnector do this.
  • Calling ExecuteQuery, ExecuteScalar, etc. when CommandText hasn't been set; expected to throw InvalidOperationException; Npgsql does not.
    • Every connector except Npgsql and System.Data.SQLite does this
  • Calling ExecuteReader when there is an open reader; Npgsql throws NpgsqlOperationInProgressException not InvalidOperationException
    • ... which is an InvalidOperationException, so I need to fix the test
  • Set DbCommand.Transaction to a transaction from a different connection should throw InvalidOperationException
  • Calling ExecuteReader when the Connection has an active transaction but the Command.Transaction property isn't set; Npgsql doesn't throw an InvalidOperationException from ExecuteReader but does throw NpgsqlOperationInProgressException from DbTransaction.Dispose.

Command

  • Npgsql throws NotSupportedException instead of InvalidOperationException when BeginTransaction` is called nested on the same connection.
  • Open throws ArgumentException not InvalidOperationException when ConnectionString isn't set.
    • Connectors are pretty inconsistent about this
roji commented

@bgrainger thanks for those! I'll take a look at them for Npgsql 3.3.

Regarding the suite itself... I think Npgsql's test suite should live in the Npgsql solution (extending your abstract test suite nuget of course) - this is the approach used in the EF Core functional tests, and as you say it would allow me to skip certain tests, modify others, etc. For the purposes of generating the comparative graphics, rather than the current method maybe you could simply include Npgsql's suite via git submodules or similar - this would avoid duplicating the suite in your project for the sole purpose of generating the comparison.

I'll try to find some time to integrate this into Npgsql soon!

roji commented

Have created a test project in the Npgsql solution which extends this, was quite easy to do. Have also started looking at some quick fixes, work is ongoing in https://github.com/roji/Npgsql/tree/spec-tests.

One point: it doesn't seem right for AdoNet.Specification.Tests to contain concrete database types (e.g. PostgresDatabaseBase... The whole point of the specific provider projects is for them to provide their own subclass of DatabaseBase. Note that in the Npgsql project I have my own subclass (MyPostgresDatabase) to override the default connection string, to make it match with what my other test suites use as a default.

Will try to report on progress...

PS Will probably submit a PR to run the latest Npgsql CI package in your comparison run - and not just the latest stable release - so we can track progress.

bgrainger commented

it doesn't seem right for AdoNet.Specification.Tests to contain concrete database types

I felt a little dirty about this, but decided that (for pragmatic reasons) it could be useful for multiple providers to be able to reuse common logic for, e.g., creating SQL for a particular server.

The ConnectionString is a hack, and almost certainly not reusable by any provider, so you're right: it should be removed.

bgrainger commented

(I didn't discuss this in the README, but a provider is of course free to implement IDbFactoryFixture from scratch, and not take a dependency on those other types.)

bgrainger commented

Removed ConnectionString in 0436b38.