Skip to content
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

Test that BCL Types work with NodaTime plugin activated #1532

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

davidroth
Copy link

Run BuiltInDataTypesNpgsqlTest again with the NodaTime plugin activated to verify that all BCL date/time types are properly supported.

Closes #1490

…ed to verify that all BCL date/time types are properly supported.

Closes npgsql#1490
@davidroth
Copy link
Author

davidroth commented Oct 12, 2020

@roji

Npgsql.EntityFrameworkCore.PostgreSQL.BuiltInDataTypesNpgsqlTest.Object_to_string_conversion fails on my machine:

image

Edit: I guess this is because my local Postgres db might use a different culture. Can this be enforced in the test-suite (e.x. by setting the culture when scaffolding the test db)?

@davidroth davidroth changed the title Test that BCL Types work with NodaTime plugin activated WIP: Test that BCL Types work with NodaTime plugin activated Oct 12, 2020
@davidroth davidroth closed this Oct 12, 2020
@davidroth davidroth reopened this Oct 19, 2020
@davidroth davidroth changed the title WIP: Test that BCL Types work with NodaTime plugin activated Test that BCL Types work with NodaTime plugin activated Oct 19, 2020
<PropertyGroup>
<EFCoreVersion>5.0.0-rc.1.20451.13</EFCoreVersion>
<MicrosoftExtensionsVersion>5.0.0-rc.1.20451.14</MicrosoftExtensionsVersion>
<NpgsqlVersion>5.0.0-ci.20201003T171056</NpgsqlVersion>
<NpgsqlVersion>5.0.0-ci.20201017T090100</NpgsqlVersion>
Copy link
Author

Choose a reason for hiding this comment

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

Needed to update to pull these changes in.

: base(fixture)
{
Fixture.TestSqlLoggerFactory.Clear();
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
Copy link
Author

Choose a reason for hiding this comment

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

Comment copied over from line 28 in main branch. Should I remove it?

@roji
Copy link
Member

roji commented Oct 21, 2020

@davidroth sorry I haven't been more reactive with all this work, am busy with other stuff... Promise to come back to this in a couple days and push it through for 5.0.

@roji
Copy link
Member

roji commented Nov 3, 2020

@davidroth am under pretty massive workload at the moment, with the 5.0 release coming up. Is there anything that isn't purely-test related, i.e. are we sure that the actual support works? If this is just about adding tests, we can always do that after 5.0 is released.

@davidroth
Copy link
Author

@davidroth am under pretty massive workload at the moment, with the 5.0 release coming up. Is there anything that isn't purely-test related, i.e. are we sure that the actual support works? If this is just about adding tests, we can always do that after 5.0 is released.

@roji This MR is only for adding tests. The tests revealed a bug first, which has already been fixed. Beside from that, I am confident, that the current support works as expected.

@roji
Copy link
Member

roji commented Nov 4, 2020

Thanks @davidroth, I'll look into this once things quiet down a bit.

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.

Update NodaTimeTests and add BCL types to verify BCL type handler forwarding
2 participants