huysentruitw / entity-framework-core-mock

Easy Mock wrapper for mocking EFCore5 DbContext and DbSet using Moq or NSubstitute
MIT License
132 stars 24 forks source link

Add possibility to set global filters on DbSet #42

Closed nevaldas closed 2 years ago

nevaldas commented 2 years ago

This PR allows others to create custom backing stores for the DbSetMock, which allows to support global filters. EF only supports one global filter, so it's safe to be able to pass only one. Because it's optional - it doesn't impact any existing users either. We have a heavy use of global filters, so we found the need for such thing, because it's bad when you can't check if your global filters work fine. This PR also adds two setups on DbContextMock. When adding/removing to/from the context, it will call appropriate DbSetMock action.

And also, usage of var setter = propertyInfo.GetSetMethod(true); in Clone function of DbSetBackingStore allows to use private setters when cloning the values. Because currently it's not possible to use private setters (which EF supports).

huysentruitw commented 2 years ago

Thank you for you PR. However, I don't think this change is benefiting this testing library, you should have a separate test for you global query filter + add a test to make sure you have applied it to your application DbContext. This testing can be done with the InMemory provider and is out of the scope of this mocking library which we want to keep as simple as possible.

This mocking library already allows you to pass your own entities for testing, so additional filtering is probably not needed.

nevaldas commented 2 years ago

Using InMemory provider is discouraged. SQLite would be an option, but having multiple dependencies and using different providers on different tests - not what we wanted, nor we wanted a new project. Yes, it means that you might be testing global filter that's not actually set (since only expression is needed to be passed). But moving expression to a static field and reusing it in both places - works just fine. This allows us to see if passing two entities (mocking what database has) only returns one. That means - if expression is working as it should. The idea of mocking library is to setup the mock to act the way you want. And I want it to filter the entities for me with given expression. It opens possibility for others to modify existing code to achieve what they need without actually modifying the library. I could remove the extra backing store I added to keep it as it was, if that's what's bothering? I never understood why this library was so closed and doesn't allow overriding stuff (like sealed classes). I could have done with reflection what I needed if not for that sealed modifier.

huysentruitw commented 2 years ago

But moving expression to a static field and reusing it in both places - works just fine. This allows us to see if passing two entities (mocking what database has) only returns one.

You don't need this library to test this, if that query is defined as IQueryable<T> MyFilter(IQueryable<T> q) a simple unit-test using would suffice.

The library is closed to avoid weird usages or recommendations in the wild which people would expect me to support. For the same reason, I'm not a fan of over-complicating this library for a 0.01% use-case, especially if what needs to be tested is not the main purpose of this library.

nevaldas commented 2 years ago

Well, but why don't allow others to extend your library with their custom code? In my case - by allowing to pass custom store? Our expression is: image And I simply pass this expression when creating the DbSet, and I can easily write a unit test to see if my expression check appropriate properties. Yes, it's a rare thing, but as said - I would be more than happy by simply being able to pass my own store.

huysentruitw commented 2 years ago

"Extension points" == "Additional maintenance" and I only have that much spare time 🙂

Write a test that compiles the expression to a Func<Location, bool> and call it with test data. There is no need to use EF or this mocking library.