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

Solution of Compute - Include - Unit test #105

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

Conversation

txavier
Copy link

@txavier txavier commented May 15, 2017

This is #53 created by @mayorovp. This pull request, though, also supplies a rewritten unit test for your review.

@txavier
Copy link
Author

txavier commented May 15, 2017

` Expected: <DelegateDecompiler.EntityFramework.Tests.EfItems.EfParent>

But was: <DelegateDecompiler.EntityFramework.Tests.EfItems.EfParent>`

I ran into possibly a bug in the test harness.

CheckerAndLogger.CompareAndLogList() is telling me that two of the object match and therefore the Assertion AreEqual fails, huh?

Can someone else (i.e. @hazzik @JonPSmith @mayorovp) confirm that this is correct?

@JonPSmith
Copy link
Collaborator

Hi @txavier,

I'm not sure what test is failing, but looking at the CheckerAndLogger.CompareAndLogList method it uses the NUnit CollectionAssert, so I doubt that that method is incorrect. Can you give a link to the specific test and also what the output is.

@txavier
Copy link
Author

txavier commented May 16, 2017

Hi @JonPSmith,

On my machine the test in the file Test01Include.cs/TestInclude() throws an exception on line 25 of CheckerAndLogger.cs/CompareAndLogList<>().

The failure message reads:
` Expected: <DelegateDecompiler.EntityFramework.Tests.EfItems.EfParent>

But was: <DelegateDecompiler.EntityFramework.Tests.EfItems.EfParent>`

So either this is a bug or the failure message is unclear.

https://github.com/hazzik/DelegateDecompiler/pull/105/files#diff-d195ae76ef87d3e981acb24f50fbf827

@JonPSmith
Copy link
Collaborator

Hi @txavier,

I do remember that NUnit's CollectionAssert isn't that friendly on its error messages. Have you run the test in debug mode with a break before the test? You could then check the two collections manually and see if they are the same.

Also, as the test is to check the Include statement you should also test the Children are the same, maybe by adding a second test or a separate test.

@txavier
Copy link
Author

txavier commented May 17, 2017

Yep, @JonPSmith upon manual checking on a breakpoint the two collections are the same.

@JonPSmith
Copy link
Collaborator

Hi @txavier,

OK, I found some time this weekend and had a look. Basically you can't use NUnit's collection assert in the way you are using it, as the collection.AreEqual for classes compares instances, not the data inside the instances (see this SO answer for how it works).

You will see that in my tests I select the primary key of the entity so that the NUnit collection assert, called by env.CompareAndLogList, is comparing integers, not classes - for example see Test05Where.

So - I would suggest you changing your test to the code below. It only checks the first Parent, but I think that is fine.

env.CompareAndLogList(linq.First().Children.Select(k => k.EfChildId).ToList(), dd.First().Children.Select(k => k.EfChildId).ToList());

PS. One small point. You should change the namespace of your test to DelegateDecompiler.EntityFramework.Tests.TestGroup20Relationship as it makes the generated docs work. See section Things to watch out for in the HowToAddMoreTests.md documentation.

@hazzik
Copy link
Owner

hazzik commented Nov 5, 2017

I had to squash commits as it was producing some strange history

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.

4 participants