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

Add UDF Db Function tests to EFCore.Relational.Specifications.Tests #12072

Merged
merged 1 commit into from
May 22, 2018

Conversation

pmiddleton
Copy link
Contributor

Add UDF Db Function tests to EFCore.Relational.Specifications.Tests per #12044

I don't have a way to test on Oracle. Will the automated tests run on Oracle?

@tuespetre
Copy link
Contributor

PAUL

Thank y’all 🤠

@@ -14,608 +12,409 @@

namespace Microsoft.EntityFrameworkCore.Query
{
public class UdfDbFunctionOracleTests : IClassFixture<UdfDbFunctionOracleTests.OracleUDFFixture>
public class UdfDbFunctionOracleTests : UdfDbFunctionTestBase<UdfDbFunctionOracleTests.OracleUDFFixture>
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Casing of OracleUDFFixture. Lets use "Udf" everywhere


public class Customer
[Fact]
public override void Scalar_Function_Extension_Method_Static()
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove redundant overrides.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there no plans to eventually check the Oracle sql?

Copy link
Member

Choose a reason for hiding this comment

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

No. Unless SQL is drastically different. We do SQL assertion for specific member/method translation which uses different function on server across provider, but if the shape is nearly same then we don't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not, unless it is different in some way that is worth verifying - we try to only verify it in one place (when we can).

@anpete
Copy link
Contributor

anpete commented May 22, 2018

Minor comments and rebase, looks good.

@pmiddleton
Copy link
Contributor Author

updated and rebased.

@smitpatel
Copy link
Member

Missing testbase override in SQLite functional tests.

@anpete
Copy link
Contributor

anpete commented May 22, 2018

@smitpatel SQLite doesn't support these tests.

@smitpatel
Copy link
Member

Then, it needs to be added to exception list. Tests are failing on it.

@anpete
Copy link
Contributor

anpete commented May 22, 2018

@pmiddleton Can you add an entry to SqliteComplianceTest.IgnoredTestBases?

@pmiddleton
Copy link
Contributor Author

@anpete - Thank you. I have been looking for 10 minutes for how that is controlled :)

@anpete anpete merged commit 8f4faa7 into dotnet:dev May 22, 2018
@anpete
Copy link
Contributor

anpete commented May 22, 2018

Thanks @pmiddleton!

@ralmsdeveloper
Copy link
Contributor

@anpete want me to try to implement these tests for Oracle?

@pmiddleton pmiddleton deleted the udfUnitTest branch May 22, 2018 19:28
@anpete
Copy link
Contributor

anpete commented May 22, 2018

@ralmsdeveloper They are there already, right?

@ralmsdeveloper
Copy link
Contributor

@anpete
You're right, I confused myself with when @pmiddleton asked.
Will the automated tests run on Oracle?

so I figured I'd have some specific test for the provider.

@pmiddleton
Copy link
Contributor Author

@ralmsdeveloper - I wasn't sure if the CI ran the Oracle tests. I don't have an Oracle instance setup to test on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants