-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Provider compliance/test suite for ADO.NET #17004
Comments
@roji |
@saurabh500 yes, that's right. Providers such as Npgsql would extend test base classes provided by you, thus running your standardized test suite. Your tests would instantiate ADO.NET objects via the provider's DbProviderFactory and make sure that the provider's DbConnection, DbCommand, etc. behave as they should. For the EFCore provider test suite, see Microsoft.EntityFrameworkCore.Specification.Tests and Microsoft.EntityFrameworkCore.Relational.Specification.Tests. The source code is here and here respectively. A good example is QueryTestBase, which runs various query scenarios and makes sure the results come back as expected. It's true that ADO.NET is a mature technology which already has well-established providers, so such a "specification" test suite might be less valuable than in EFCore, which is totally new technology where providers can use the specification while implementing. But even with ADO.NET it would be very useful to automatically find divergences in provider behavior, leading to either standardized behavior or at least explicit documentation of those divergences. |
Note that Npgsql contains many tests which are completely generic - i.e. aren't specific to Npgsql/PostgreSQL in any way. It is exactly this kind of tests that belong in a provider-independent specification test library. |
@roji do you want to grab this task? |
@saurabh500 I have a lot on my plate, but I can try to spend some time on this (not right away though). Also, I'm pretty PostgreSQL-centric and will definitely need help with making sure things are portable. |
Sounds good. |
Just to say that I'm unlikely to find any time to work on this soon... Hopefully Microsoft (or someone else) can pick this up. |
cc @bricelam what do you think about refactoring the Microsoft.Data.Sqlite test to follow the test spec pattern we already use for EF Core provider tests? |
They weren't written with that purpose in mind, but it could be a good starting point since they're very functional in nature. |
@bricelam I think that is probably true for (at least some of) the SqlClient tests. |
This is something I've been interested in having for a while, too (as the author of MySqlConnector). I got tired of manually writing test cases to check compatibility across different drivers, so I created an automated test harness. 🙂 A WIP rough draft is here: https://github.com/mysql-net/AdoNetApiTest I bootstrapped the prototype with @bricelam's SqliteCommand tests (which saved a lot of time, thanks!). Inspired by http://seriot.ch/json/parsing.html, it dumps output like the following. (Take the colours in the cells with a grain of salt; I haven't investigated them all to ensure that they're not caused by a bug in the test harness itself.) |
@bgrainger that looks like a wonderful start - I'm excited to see this. Although I expect to be too busy until end of December, I definitely look forward to contributing to this and evolving it. In the meantime, please take a look at the Npgsql tests - a lot of them are exactly the kind of generic, functional tests that would fit perfectly into this. One more comment - while it's really useful for this to exist as a self-contained project that includes connectors to specific ADO.NET providers, it's also important that projects like Npgsql and MySqlConnector be able to use this in their solutions, and to override certain things. In other words, 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. |
Thanks; that's a great suggestion (and I hadn't thought that far ahead yet...). Created mysql-net/AdoNetApiTest#3 to discuss it (without hijacking this issue). |
Note suggestion for doing something similar with performance - a standardized ADO.NET benchmark suite aspnet/DataAccessPerformance#37. |
I spent part of last week adding an Azure Pipeline to run the tests I have so far in AdoNetApiTest against MySQL, PostgreSQL, SQLite, and SQL Server in Docker containers. The latest results then get automatically published here. This is a quick way to see at-a-glance how a particular provider aligns with others, or what is the most common behaviour. (There is still a NuGet package--not updated in a long time--with these same tests that providers can integrate into their CI builds to check for regressions via a reusable test suite.) Possible future enhancements include generating a unique URL for each Azure Pipeline build so there's a permalink for any particular run. |
This is looking very, very nice... Thanks for doing it @bgrainger! Long term, I still believe that these compliance tests should be part of each provider's solution rather than centralized - but it's definitely great to have something today until that happens. /cc @YohDeadfall @austindrenski - time permitting, we should try to fix some of these, at least the low-hanging fruit. No need for regression tests though :) |
Completely agree. This is not intended to replace the use of a common test suite by individual providers; in fact, it's expected that providers will customise the tests (e.g., here in MySqlConnector) to override the default results where they don't make sense, and the AdoNetApiTest solution couldn't pull in all those customisations (nor should they be authored there). This is simply a quick way to see at a glance if there's consensus across different providers for what the most reasonable behaviour is in situations where the documentation isn't clear. (As I wrote above, I started this from my own desire to automate ad-hoc manual testing I was already doing to try to choose the most common behaviour when there were multiple alternatives.) |
Great. By the way, your project could still pull in the compliance suite from the various providers via git submodules and continue to display the same comparative table (which is really useful). As more providers add these, the project could switch to using them one by one. |
I considered that, but wasn't sure if (when using a submodule) I could still force the latest version of AdoNet.Specification.Tests (from that repo) to be loaded, instead of a version from NuGet. (I.e., how to change the project in the submodule from using a PackageReference to a project reference.) Being able to add new tests and immediately see their results across all providers is rather useful. |
That's a good point. It's probably possible to do some awful MSBuild hack to change the PackageReference into a ProjectReference, but it ain't gonna be pretty. |
No fixes/changes to Npgsql itself - some tests are failing. No CI. Part of npgsql#2225 See also https://github.com/dotnet/corefx/issues/7810
Some less trivial scenarios still failing. Part of npgsql#2225 See also https://github.com/dotnet/corefx/issues/7810
No fixes/changes to Npgsql itself - some tests are failing. No CI. Part of #2225 See also https://github.com/dotnet/corefx/issues/7810
Some less trivial scenarios still failing. Part of #2225 See also https://github.com/dotnet/corefx/issues/7810
Context: dotnet/runtime#74459 Context: dotnet/runtime#71004 Context: dotnet/runtime#54845 dotnet/runtime#54845 added support TimeZoneInfo support for Android. This was found to slow down initial use of `DateTimeOffset.Now` by ~277ms (dotnet/runtime#17004). dotnet/runtime#74459 started reducing this startup overhead by adding support for a new `System.TimeZoneInfo.LocalDateTimeOffset` property on `AppContext`: if `System.TimeZoneInfo.LocalDateTimeOffset` is set, it is used as the initial local DateTime Offset value. Update `mono.android.MonoPackageManager.LoadApplication()` and `libmono-android*.so` so that the `System.TimeZoneInfo.LocalDateTimeOffset` AppContext property is set during process startup. This can reduce `DateTimeOffset.Now` startup overhead from ~277ms to ~50ms. Note: `System.TimeZoneInfo.LocalDateTimeOffset` is fastest on API-26+ (Android 8.0+) targets, as it uses [`java.time.ZoneOffset.getTotalSeconds()`][0]. Note: `$(AndroidJavaRuntimeApiLevel)` is updated to API-26 so that `ZoneOffset.getTotalSeconds()` can be used. Care should be taken within `src/java-runtime` to ensure that Android API use is appropriately protected behind runtime version checks. [0]: https://developer.android.com/reference/kotlin/java/time/ZoneOffset#gettotalseconds
Context: dotnet/runtime#74459 Context: dotnet/runtime#71004 Context: dotnet/runtime#54845 dotnet/runtime#54845 added support TimeZoneInfo support for Android. This was found to slow down initial use of `DateTimeOffset.Now` by ~277ms (dotnet/runtime#17004). dotnet/runtime#74459 started reducing this startup overhead by adding support for a new `System.TimeZoneInfo.LocalDateTimeOffset` property on `AppContext`: if `System.TimeZoneInfo.LocalDateTimeOffset` is set, it is used as the initial local DateTime Offset value. Update `mono.android.MonoPackageManager.LoadApplication()` and `libmono-android*.so` so that the `System.TimeZoneInfo.LocalDateTimeOffset` AppContext property is set during process startup. This can reduce `DateTimeOffset.Now` startup overhead from ~277ms to ~50ms. Note: `System.TimeZoneInfo.LocalDateTimeOffset` is fastest on API-26+ (Android 8.0+) targets, as it uses [`java.time.ZoneOffset.getTotalSeconds()`][0]. Note: `$(AndroidJavaRuntimeApiLevel)` is updated to API-26 so that `ZoneOffset.getTotalSeconds()` can be used. Care should be taken within `src/java-runtime` to ensure that Android API use is appropriately protected behind runtime version checks. [0]: https://developer.android.com/reference/kotlin/java/time/ZoneOffset#gettotalseconds
Context: dotnet/runtime#74459 Context: dotnet/runtime#71004 Context: dotnet/runtime#54845 dotnet/runtime#54845 added support TimeZoneInfo support for Android. This was found to slow down initial use of `DateTimeOffset.Now` by ~277ms (dotnet/runtime#17004). dotnet/runtime#74459 started reducing this startup overhead by adding support for a new `System.TimeZoneInfo.LocalDateTimeOffset` property on `AppContext`: if `System.TimeZoneInfo.LocalDateTimeOffset` is set, it is used as the initial local DateTime Offset value. Update `mono.android.MonoPackageManager.LoadApplication()` and `libmono-android*.so` so that the `System.TimeZoneInfo.LocalDateTimeOffset` AppContext property is set during process startup. This can reduce `DateTimeOffset.Now` startup overhead from ~277ms to ~50ms. Note: `System.TimeZoneInfo.LocalDateTimeOffset` is fastest on API-26+ (Android 8.0+) targets, as it uses [`java.time.ZoneOffset.getTotalSeconds()`][0]. Note: `$(AndroidJavaRuntimeApiLevel)` is updated to API-26 so that `ZoneOffset.getTotalSeconds()` can be used. Care should be taken within `src/java-runtime` to ensure that Android API use is appropriately protected behind runtime version checks. [0]: https://developer.android.com/reference/kotlin/java/time/ZoneOffset#gettotalseconds
To promote standard behavior across ADO.NET providers, it would be great if a test suite existed. Ideally all provider-independent tests would be extracted from SqlClient and made available via a nuget. This would be similar to what the EF Core team has done.
The text was updated successfully, but these errors were encountered: