Mocking a repository



  • So I have a repository class, which snipped looks kinda like this:

    public class Repository<T> : IRepository<T>
    {
        public IQueryable<T> GetAll()
        {
            //snip
        }
    
        public IQueryable<T> Query<T>(Expression<Func<T, bool>> query) => GetAll().Where(query);
    }
    

    There's a bunch of other helper methods too, some less complex, some a bit more. They all use GetAll() and apply stuff to it.

    I also have a service under test which uses Query<T>:

    var result = repository.Query(x => x.Foo.Bar == Baz.Quux)
    

    Now, when I want to unit test that service, I obviously want to mock the repository somehow. Preferably with some impromptu test data to be returned.

    Now, I can either:

    • mock the Query method and have it always return a constant list. But that doesn't test whether that x => x.Foo.Bar == Baz.Quux condition filters the data properly.
    • mock the Query method and have it return a constant list when passed this exact condition. But now I've gone the other way - the test depends on the condition being exactly this, not on the condition filtering the data right.
    • mock the Query method, create a general list representing all rows of the DB and call that Where() myself in the mock. But that's reinventing the wheel - I have to pretty much reimplement all my helpers in my unit tests.
    • mock the GetAll method and have the repository worry about filtering data. But that means if Query() ever gets rewritten not to use GetAll(), the unit tests will start failing for no reason other than an implementation change in an underlying layer.

    Which of those options is the most sensible? Or is there any other one I don't see?


  • Discourse touched me in a no-no place

    I'd mock GetAll() so that the unit tests would be running against a simple, known data model. Because anything else is likely to be annoying.



  • @dkf said in Mocking a repository:

    Because anything else is likely to be annoying.

    Though now you have to ensure Query calls GetAll, which is very much not a part of the repository's specification. Probably not a huge problem, since I don't see a good reason not to do it this way, but it still seems dirty.

    IOW, it's probably the most pragmatic solution, but I wonder what the "proper" one would be.



  • Can you go one level deeper and mock whatever is providing the data to GetAll?
    That would probably be the cleanest solution, since I assume it will be in the spec and won't change.


  • I survived the hour long Uno hand

    What are you testing? You don't mock the unit you're testing. You do mock everything else.

    @Maciejasjmj said in Mocking a repository:

    mock the Query method and have it always return a constant list. But that doesn't test whether that x => x.Foo.Bar == Baz.Quux condition filters the data properly.

    Don't do this. That's not testing your method.

    @Maciejasjmj said in Mocking a repository:

    mock the Query method and have it return a constant list when passed this exact condition. But now I've gone the other way - the test depends on the condition being exactly this, not on the condition filtering the data right.

    Ditto.

    @Maciejasjmj said in Mocking a repository:

    mock the Query method, create a general list representing all rows of the DB and call that Where() myself in the mock. But that's reinventing the wheel - I have to pretty much reimplement all my helpers in my unit tests.

    I'm not sure I even understand this.

    @Maciejasjmj said in Mocking a repository:

    mock the GetAll method and have the repository worry about filtering data. But that means if Query() ever gets rewritten not to use GetAll(), the unit tests will start failing for no reason other than an implementation change in an underlying layer.

    This makes some sense: your unit tests are supposed to be testing the implementation, not the black-box interface per se.

    If your code is literally

    @Maciejasjmj said in Mocking a repository:

    public IQueryable<T> Query<T>(Expression<Func<T, bool>> query) => GetAll().Where(query);
    

    (and I'll take your word for it that you need this method at all, if it can be replaced with a single one-liner? Because I don't .NET anymore, but I don't understand what you're doing here or why, it seems like you're re-inventing LINQ)

    then your tests should be something like (using Mocha because I can't be arsed to look up nUnit again)

    describe('Query', () => {
      it('should call getAll()', () => {
        sinon.stub(repository, 'getAll').returns(predefinedList);
        repository.Query(x => x.Foo.Bar == Baz.Quux);
        repository.getAll.should.have.been.called;
      });
    
      it('should apply a condition', () => {
        sinon.stub(repository, 'getAll').returns(predefinedList);
        var result = repository.Query(x => x.Foo.Bar == Baz.Quux);
        result.should.contain(item1);
        result.should.not.contain(item2);
      });
    });
    

    That's it, you're done. The method does two things, and you checked them both. The implementation of Where is out of scope for your unit, because you didn't write that, it's LINQ standard.

    The hardest part of unit testing is sometimes figuring out :wtf: you're testing.



  • @Yamikuronue said in Mocking a repository:

    hardest part of unit testing is sometimes figuring out :wtf: you're testing.

    He doesn't want to test the repository class, he wants to test things that depend on the repository class.


  • I survived the hour long Uno hand

    @NedFodder Ah.

    @Maciejasjmj said in Mocking a repository:

    mock the Query method and have it return a constant list when passed this exact condition. But now I've gone the other way - the test depends on the condition being exactly this, not on the condition filtering the data right.

    Do that, but check that the right condition was passed in using your mock.

    What you really want is an integration test: will your business logic work with real data? Without knowing what the service does, I'd construct specific sets of data that test specific edge cases, load them into a repository, and test the service calling against that repository.



  • @Yamikuronue said in Mocking a repository:

    The hardest part of unit testing is sometimes figuring out :wtf: you're testing.

    Yeah... I think I wasn't clear. I'm not testing the repository. That's got its own test suite, and in fact this test:

    describe('Query', () => {
      it('should call getAll()', () => {
        sinon.stub(repository, 'getAll').returns(predefinedList);
        repository.Query(x => x.Foo.Bar == Baz.Quux);
        repository.getAll.should.have.been.called;
      });
    

    is what I think we shouldn't be doing - enforcing the implementation via unit tests. The repository should ideally be free to do whatever it wants internally as long as Query() produces correct results.

    The problem is more like this. I have a service:

    public class Service
    {
        private IRepository<Model> repository; //DI'd via constructor
    
        public IEnumerable<Model> GetData()
        {
             return repository.Query(x => x.Foo.Bar == Baz.Quux); //and some other things
        }
    }
    

    I want to unit test this service. Approach one:

    [Test]
    public void Test1()
    {
        var result = new List<Model>(...).AsQueryable();
        var mockRepository = new Mock<IRepository<Model>>();
        mockRepository.Setup(x => x.Query(It.IsAny<Expression<Func<Model, bool>>>)).Returns(result); //just return result whenever Query is called
        var data = (new Service(mockRepository)).GetData(); //and verify
    }
    

    2:

    [Test]
    public void Test2()
    {
        var result = new List<Model>(...).AsQueryable();
        var mockRepository = new Mock<IRepository<Model>>();
        mockRepository.Setup(x => x.Query(y => y.Foo.Bar == Baz.Quux)).Returns(result); //return result when given the condition from Service's implementation (does that even work?)
        var data = (new Service(mockRepository)).GetData(); //and verify
    }
    

    3:

    [Test]
    public void Test1()
    {
        var result = new List<Model>(...).AsQueryable();
        var mockRepository = new Mock<IRepository<Model>>();
        mockRepository.Setup(x => x.Query(It.IsAny<Expression<Func<Model, bool>>>)).Returns(x => result.Where(x)); //return result and call Where() on it manually
        var data = (new Service(mockRepository)).GetData(); //and verify
    }
    

    4

    [Test]
    public void Test1()
    {
        var result = new List<Model>(...).AsQueryable();
        var mockRepository = new Mock<Repository<Model>>() { CallBase = true }; //call Repository's real functions - now we need to mock the implementation instead of the interface
        mockRepository.Setup(x => x.GetAll()).Returns(result); //GetAll returns the whole list every time, and it's up to Repository's implementation details to make sure that list actually gets used
        var data = (new Service(mockRepository)).GetData(); //and verify
    }
    


  • @Yamikuronue said in Mocking a repository:

    What you really want is an integration test: will your business logic work with real data?

    I want a unit test really, integration tests are a wholly different can of galoshes that will likely be hitting a real database to prevent EF pooping itself when some IQueryable features aren't implemented in the SQL Server provider.

    I'm honestly not sure how I'd go about unit testing methods that, in 90% of the cases, are nothing but queries on the underlying datastore without having some data in said datastore. I guess I could just run it and check it doesn't throw, but that's not much...


  • kills Dumbledore

    @Maciejasjmj Assuming the repository already has unit tests, don't you want to mock it out so repository.Query returns what you would expect it to? Then the unit test is not doing much at the moment but resists regressions if GetData() is modified in the future and suddenly isn't just a query on the underlying data



  • @Jaloopa said in Mocking a repository:

    don't you want to mock it out so repository.Query returns what you would expect it to?

    Yeah, but I'd still need a test that actually tests the lambda Query's been passed. Otherwise I could just have Query(x => (new Random()).Next(100) > 50) and it'd still pass all the tests.

    well, except for LINQ-to-Entities throwing up on that, but you know what I mean


  • I survived the hour long Uno hand

    What is the responsibility of the service? What is the responsibility of the repository?

    It seems like nowhere do you have a strong data model, everywhere you have little bits and pieces of data. So you're struggling to figure out how to mock out half your model so you can test the other half.

    I want a unit test really, integration tests are a wholly different can of galoshes

    Integration tests cross unit boundaries; unit tests don't. If you want to test two units working in tandem, you want an integration test. But this is sounding more and more like they should be a single unit in the first place.



  • @Yamikuronue said in Mocking a repository:

    What is the responsibility of the repository?

    It's basically a thin, generic wrapper over Entity Framework's DbSet<T>. It concerns itself with a single database table and exposes some helper methods for now.

    Honestly we'd just be using Entity Framework, except one of the requirements is that in the futere there's gonna be a role-permission setup built into the DAL - a role can, for example, only have read permissions to a particular table, or only be able to read records with a particular property, and it's supposed to be checked before services get their hands into the data. So for example when you call GetAll() and your role says you can only see records with your username, you'll transparently only get records with your username. I'm not too proud of this, but that's how we - and the client - roll.

    The services actually do business logic - they take one or more repositories as dependencies, and actually determine what to pull from or insert into the database. There's a whole lot of services strewn among different assemblies, but they all use the same generic repository implementation.


  • I survived the hour long Uno hand

    @Maciejasjmj said in Mocking a repository:

    I'm honestly not sure how I'd go about unit testing methods that, in 90% of the cases, are nothing but queries on the underlying datastore without having some data in said datastore. I guess I could just run it and check it doesn't throw, but that's not much...

    @Maciejasjmj said in Mocking a repository:

    It's basically a thin, generic wrapper over Entity Framework's DbSet<T>. It concerns itself with a single database table and exposes some helper methods for now.

    Basically, your repository is your answer to your earlier question: it essentially is the database now. Mock it, fill it with curated data, and ensure that the service pulls the right thing. If your business logic says "only return birds from this service", mock getAll() to return [pidgey, spearow, growlithe, pikachu] and assert that only pidgey, spearow are returned. That sort of thing. That'll test that your query is logically sound without asserting what it should be (and thus duplicating the work).

    You're going to have to break the seal a little on the repository to do this, though, because otherwise you're re-implementing your query method. But making major changes like "query no longer calls getAll" sounds like it should issue a warning: your permission logic will be in getall, you don't want to change that dependency chain. Broken unit tests are more of a warning than an error anyway, because they only matter if you didn't expect them to break.


  • Discourse touched me in a no-no place

    @Yamikuronue said in Mocking a repository:

    Broken unit tests are more of a warning than an error anyway, because they only matter if you didn't expect them to break.

    THIS! It's ever so important to remember what you're testing and to just test that, not some other random system written by someone else.



  • @Maciejasjmj said in Mocking a repository:

    I'm honestly not sure how I'd go about unit testing methods that, in 90% of the cases, are nothing but queries on the underlying datastore without having some data in said datastore. I guess I could just run it and check it doesn't throw, but that's not much...

    I'm sure some pedantic dickweed is going to come by and say "well it's not a unit test then!" but whatever.

    At our company, we just keep a portable SQL database with enough schema and sample data we can use for testing and attach it before the test run. Whether or not it's a "unit test" technically, Visual Studio's test runner is perfectly capable of this and it works fine.



  • @Maciejasjmj said in Mocking a repository:

    It's basically a thin, generic wrapper over Entity Framework's DbSet<T>. It concerns itself with a single database table and exposes some helper methods for now.

    Why don't you just use...

    @Maciejasjmj said in Mocking a repository:

    Honestly we'd just be using Entity Framework, except one of the requirements is that in the futere there's gonna be a role-permission setup built into the DAL

    Oh.

    I think before you worry about unit tests, maybe you should spent some time worrying about YAGNI. (You Ain't Gonna Need It.) Why are you contorting yourself to support a feature you might possibly have in the future?

    Especially because when/if that future date arrives, you might implement the feature in a totally different way (perhaps by populating the EF from a View or Stored Procedure that already does the permissions filtering, which would be a better way to do this ANYWAY), completely invalidating the contortions you're doing now to make it work the way you predict it might. In the future. Possibly.


  • Winner of the 2016 Presidential Election

    @Maciejasjmj said in Mocking a repository:

    one of the requirements is that in the futere there's gonna be a role-permission setup built into the DAL

    @blakeyrat said in Mocking a repository:

    I think before you worry about unit tests, maybe you should spent some time worrying about YAGNI. (You Ain't Gonna Need It.) Why are you contorting yourself to support a feature you might possibly have in the future?

    Maciejasjmj We have a requirement for functionality that we will definitely need in the future.
    blakeyrat YAGNI

    But he just said...



  • @error He needs the feature. He doesn't need it to be built into the DAL. Which, IMO, is the entirely wrong place to put it.

    Unless he has a client who specifically says, "put the permissions system in the DAL and not in the database," in which case he needs to fire clients who dictate requirements like that. Because it totally ties your hands.



  • EDIT: Meh, everyone else has better answers.


  • Impossible Mission - B

    Ha ha! Look how ugly that repository is!

    ...wait, that's not what you meant?


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.