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

Fixes migration unit tests #293

Merged
merged 5 commits into from
Jun 16, 2017
Merged

Fixes migration unit tests #293

merged 5 commits into from
Jun 16, 2017

Conversation

yukozh
Copy link
Member

@yukozh yukozh commented Jun 16, 2017

No description provided.

@yukozh yukozh requested a review from caleblloyd June 16, 2017 17:12
@yukozh
Copy link
Member Author

yukozh commented Jun 16, 2017

@caleblloyd Did the UT in your machine can pass 43 cases? or you can only pass 7 cases now? The EF core always connect to a incorrect connection string to get the mysql version.

@caleblloyd
Copy link
Contributor

I think I need to switch the Connection Settings to do a lazy initialize

@yukozh
Copy link
Member Author

yukozh commented Jun 16, 2017

In this way, the migrations will be mitigated now, but on update will be broken. I will keep investigate the next days. I was too busy in previous term to maintain this project. Thanks for your contributions these days.

@caleblloyd
Copy link
Contributor

@kagamine switched ConnectionSettings to lazy initialize in 2ea2961 you can merge the dev branch to get the fix.

Now the dev branch UT is:

Total tests: 57. Passed: 43. Failed: 13. Skipped: 1.

@yukozh
Copy link
Member Author

yukozh commented Jun 16, 2017

@caleblloyd Now the UT fails 3 cases only. I want to checkin this first and fix them later. What's your thoughts?

$"Error in {table}.{name}: DATETIME does not support values generated " +
"on Add or Update in MySql <= 5.5, try explicitly setting the column type to TIMESTAMP");
}
catch {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not catch and ignore this. Instead we should mock ConnectionSettings in the UT so it doesn't fail

Copy link
Member Author

Choose a reason for hiding this comment

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

reverted

@caleblloyd caleblloyd merged commit 414ce69 into dev Jun 16, 2017
@yukozh
Copy link
Member Author

yukozh commented Jun 16, 2017

Removed try catch will throw an exception, could you investigate this? @caleblloyd

@caleblloyd
Copy link
Contributor

Yes I'll work up a mock options

@yukozh
Copy link
Member Author

yukozh commented Jun 17, 2017

@caleblloyd Does pomelo 2.0 not work?

I've gotten an exception:

Unhandled Exception: System.ArgumentNullException: Value cannot be null.
Parameter name: contextOptions

@caleblloyd
Copy link
Contributor

@kagamine that's right it does not work yet. I'm tracking issues with upstream as I hit them in #288

@yukozh yukozh deleted the fix-migraion-2-0-0 branch July 1, 2017 13:43
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.

2 participants