Some test simplifications #4989
Conversation
| public async Task Ranges_not_supported_with_slim_builder_without_EnableRanges() | ||
| { | ||
| await using var slimDataSource = new NpgsqlSlimDataSourceBuilder(ConnectionString).Build(); | ||
| await AssertTypeUnsupportedRead<NpgsqlRange<int>, InvalidCastException>(@"[1,2)", "int4range", slimDataSource); |
There was a problem hiding this comment.
#4971 is going to make it so we throw NotSupportedException if we attempt to read a record without EnableRecords. We probably should align them (return unsupported handler for ranges?)
There was a problem hiding this comment.
Absolutely, that would be great. This way the user also gets the informative message when they attempt to read a range.
There was a problem hiding this comment.
Probably in a separate pr. Also, might be a good idea to make a separate resolver with all of the unsupported types (instead of adding them to BuiltInTypeResolver)
There was a problem hiding this comment.
👍 for separate PR. Any particular reason for the separate resolver?
There was a problem hiding this comment.
- It will make BuiltInTypeResolver a bit faster (for example switch over dataTypes might be a bit more optimized since there will be less values)
- Just having all of them bundled in a separate file is nice, BuiltInTypeResolver only handles things it supports, UnsupportedTypeResolver only handles things it doesn't support
There was a problem hiding this comment.
OK, I doubt there will be a noticeable perf improvement, but why not.
roji
force-pushed
the
TestStuff
branch
2 times, most recently
from
2db3f5a to
7a28f86
Compare
|
This was already merged in a while ago via #5279 |
3 participants
No description provided.