Skip to content

Conversation

@SphtKr
Copy link
Collaborator

@SphtKr SphtKr commented Feb 9, 2015

GetKeyNames was completely broken, actually: it was always returning only Id, and swallowing an exception on every run. Now it properly checks for several conditions, and has tests to back it up.

Also fixed TestEntities to be able to delete the DB and recreate the model if it changes (which it did in this commit).

@SphtKr
Copy link
Collaborator Author

SphtKr commented Feb 9, 2015

Trying to make pull requests for my own changes...still getting used to really using Git and GitHub.

@SphtKr SphtKr added this to the 0.3.0 milestone Feb 9, 2015
@SphtKr SphtKr added the bug label Feb 9, 2015
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like FluentAssertions for this sort of thing. You can do:

Action action = () =>
{
    IEnumerable<string> keyNames = materializer.GetKeyNames(typeof(NotAnEntity));
};
action.ShouldThrow<ArgumentException>();

which I think is clearer than adding an attribute on the method. You can also add additional checks against the exception object itself, which you can't do with the MSTest approach.

@csantero
Copy link
Collaborator

csantero commented Feb 9, 2015

Oh yeah, I think I ran into this the other day. 👍

SphtKr added a commit that referenced this pull request Feb 9, 2015
@SphtKr SphtKr merged commit f179e80 into master Feb 9, 2015
@SphtKr SphtKr mentioned this pull request Feb 10, 2015
@csantero csantero deleted the ef-materializer-nonstandard-id-bug branch February 20, 2015 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants