Library does not lend itself to unit testing

[cidthecoatrack]

I am attempting to implement unit tests of a repository that uses NRedisGraph to access my Redis data. As one should always do for unit testing, I am mocking/faking al ldependencies, which includes objects from NRedisGraph. However, the library is too hard-coded and makes this impossible:

1. Because the only way to create the graph object is via a new command (no abstract factory, no interface for dependency injection, etc.), I had to wrap the graph object in an adapter so that it would be unit testable.
2. My query returns the ResultSet from the graph object's QueryAsync function. However, this object is sealed and internal. However, since it implements IEnumerable<Record>, I could create that enumerable and cast it to ResultSet when needed. Further cumbersome, but at least doable.
3. The constructors for the Record are all internal, so I cannot instantiate a new one when setting up my mocks in my test. The class is sealed, which means I cannot extend it. It implements no interfaces and isn't abstract, so I can't mock it directly. This means that I cannot, in any way, shape, or form, create a Record object in a unit-test appropriate way. Therefore, all the other earlier work is for naught.

Ideally, we would have interfaces for the RedisGraph object, so it can be mockable at the root. Barring that, there needs to at least be 1 public constructor for the Record object, so they can be instantiated for mocks in a unit test.

Below is an example of my test:

        [Test]
        public async Task GetUserOrganizationPoliciesAsync_SingleScope_ReturnsUserPolicies()
        {
            //Arrange
            var mockGraph = new Mock<IRedisGraphAdapter>();
            _mockRedisGraphFactory.Setup(f => f.BuildAsync(0)).ReturnsAsync(mockGraph.Object);

            var mockRecord = new Mock<Record>();
            var policies = new[] { "my:policy", "my:other:policy" };
            mockRecord.Setup(r => r.GetString("r.policies")).Returns(JsonConvert.SerializeObject(policies));

            IEnumerable<Record> records = new[]
            {
                mockRecord.Object,
            };

            var queries = new Dictionary<string, IDictionary<string, object>>();
            mockGraph
                .Setup(g => g.QueryAsync("auth", It.IsAny<string>(), It.IsAny<Dictionary<string, object>>()))
                .Callback((string d, string q, IDictionary<string, object> p) => queries[q] = p)
                .ReturnsAsync((ResultSet)records);

            //Act
            var result = await _userPolicyCacheRepository.GetUserOrganizationPoliciesAsync(9266, 90210);

            //Assert
            Assert.That(result.ErrorMessage, Is.Null);
            Assert.That(result.Success, Is.True);
            Assert.That(result.Policies, Is.EqualTo(policies));

            mockGraph.Verify(g => g.QueryAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<Dictionary<string, object>>()), Times.Once);
            var policyQuery = "MATCH (r:Role)-[:ASSIGNED_TO { user_id: $userId }]-(s:Scope)-[:INHERITS_FROM*0..3]->(:Scope { id: $scopeId }) " +
                               "RETURN r.policies " +
                               "UNION " +
                               "MATCH (:Scope { id: $scopeId })-[:INHERITS_FROM*0..3]->(s:Scope)-[:ASSIGNED_TO { user_id: $userId }]-(r:Role) " +
                               "RETURN r.policies";

            Assert.That(queries, Has.Count.EqualTo(1)
                .And.ContainKey(policyQuery));
            Assert.That(queries[policyQuery], Has.Count.EqualTo(2)
                .And.ContainKey("scopeId")
                .And.ContainKey("userId"));
            Assert.That(queries[policyQuery]["scopeId"], Is.EqualTo(90210));
            Assert.That(queries[policyQuery]["userId"], Is.EqualTo(9266));
        }

[tombatron]

Pull requests are welcome!

[tombatron]

So I've reviewed this question, and the companion pull request, and I have some observations.

In the example test provided above, it appears as if two questions are under consideration:

1. Does the repository execute a graph query and get an expected response?
2. Does the query that the repository sends to RedisGraph match the expected query that (I'm assuming) is defined in the repository?

The first area under test seems better suited to an integration test with appropriate test data. My thinking here stems from the structure of that part of the test. If you notice, it looks like the mocking framework is being exercised and not application logic. In this case, an integration test would allow you to validate that the graph query defined is syntactically correct and returns an expected result set which would obviate the need for the second area of the unit test (which is more of an assertion of the implementation rather than a test of logic).

With all of that said, I really appreciate your contribution but I don't think the suggested changes are appropriate for this library.

Cheers,

Tom

[cidthecoatrack]

I agree that this absolutely would work better as a low-level integration test - that would in fact be my normal way to test a repository like this. However, in this particular circumstance, that isn't an option. The particulars of the test aside, the fact that the library is not abstractable makes unit testing anything that uses it very difficult. So even if I wasn't going to test graph directly as I do in my example test, because I cannot inject/abstract RedisGraph (without wrapping it in my own adapter/factory, as I have done as a mitigating measure), the entire class becomes untestable, which is simply a bad architecture.

Ultimately, it's your choice, as it's your library/package and therefore your architecture. I appreciate the time you've spent looking at it.