When Unit testing, Is using a the verify to indirectly test a method ok in this example?
c#, mocking, unit-testing
Solution
Testing private methods through a public method is fine. If your code is modular enough there shouldn't be too much setup code in order get the conditions correct to get into your private method. If you do find yourself setting up a lot of stuff just to get into your private method, chances are you're doing too much in one class.
In your case, I would be tempted to go with your point 3), and create a PolicyFinder : IPolicyFinder class. Maybe you don't need to re-use it right now, be it makes your code much easier to modify in the future and makes both classes easier to test
(See http://en.wikipedia.org/wiki/Single_responsibility_principle)
edit: I didn't fully read your 3) bullet point, so sorry for hitting you with the single responsibility stick ;)
Problem
I would like to test FindPolicyToBeResent() in the following piece of code. I have a few options available but would like to know how anybody else would approach this situation also if my approach ie the last option is ok? - Make FindPolicyToBeResent() public. This not an option as it exposes an implementation for the sole reason of testing and makes interfaces confusing - Unit test using the public API only but this could be difficult as I never expose the collection that I am filtering directly in the return statement and cannot do so for security reasons. This would mean that I could only have limited tests - Usually I would break the code out into a new object in this situation but it doesn't feel right in this case, I cant foresee this filter being reused anywhere else in the system so it would be no better than making the method public and before anyone hits me with the single responsibility stick (which would be justified), coding is a balancing act, I feel like it would be breaking the Keep It Simple principle. It just feels like making a class to service the test rather than it actually having a seperate single responcibility. Plus it results in file and class bloat. - I could make the code an extension method of the IEnumerable which would make it testable but again I cant forsee this filter being used anywhere else so it would make more sense if it remained in this class - The last option and prefer but may be seen as a bit of a hack is test the mock of documentResenderRepository.CanPolicyBeResent(policy.Id) further down the code using verify() to see how many time it has bee hit. I am not sure if this is a good idea? Thoughts? My preference is for the last option but it does feel a bit dirty, I have an example at the bottom ``` public class DocumentResendService : IDocumentResendService { #region Member Variables ... #endregion #region Constructors ... #endregion #region Accessors ... #endregion #region Methods public IDocumentResendResponse ResendDocuments() { if (!IsInputsValid()) { return response; } RecordRequestAttempt(); if (RequestLimitIsReached()) { return response; } FindPolicyToBeResent(); if(PolicyCanNotBeResent()) { return response; } RequestDocumentToBeResent(); return response; } private bool IsInputsValid() { .. } private void RecordRequestAttempt() { ... } private bool RequestLimitIsReached() { ... } // I want to test this method which basically just filters the policies private void FindPolicyToBeResent() { var allPolicies = policyDocumentRepository.GetPolicy(AgentCode, Email, PostCode, SearchDate, BirthDate); policies = allPolicies.Where(currentPolicy => currentPolicy.IsActive() || currentPolicy.IsInTheFuture()); if (policies.Count() == 0 ) { policies = allPolicies.FilterToPolicyThatHasEndedMostRecently(); } } private bool PolicyCanNotBeResent() { if (policies == null || !policies.Any()) { response.Add(ErrorCode.PolicyCanNotBeFound); return true; } foreach (var policy in policies) { // I could mock this line and use a verify here which policy id's are passed in if (documentResenderRepository.CanPolicyBeResent(policy.Id) == false) { response.Add(ErrorCode.UnableToResendDocument); } } return response.Errors.Any(); } private void RequestDocumentToBeResent() { ... } #endregion } ``` Here is the unit test solution for the last option but ``` [TestFixture] public class FindPolicyToBeResentTest : DocumentResenderTestsBase { private readonly List<Policy> allPolicies = new List<Policy>(); public FindPolicyToBeResentTest() { var day = -250; for (int i = 1; i < 6; i++) { var policy = new Policy { Id = i, StartDate = DateTime.Now.AddDays(day) }; day = day + 100; policy.EndDate = DateTime.Now.AddDays(day); allPolicies.Add(policy); } } private void SetUpDocumentResender(IEnumerable<Policy> policies) { SetUpObjectDefaultsForDocumentResenderCreation(); policyRepositoryMock.Setup(y => y.GetPolicy(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<DateTime>(), It.IsAny<DateTime>())) .Returns(policies); documentResendService = CreateDocumentResendService(); SetDocumentResenderDefaults(); } [Test] public void PoliciesThatAreNotActiveOrAreInThePastShouldBeFilteredOut() { SetUpDocumentResender(allPolicies); documentResendService.ResendDocuments(); foreach (var policy in allPolicies) { if (policy.IsActive() || policy.IsInTheFuture()) { documentResenderRepositoryMock.Verify(x => x.CanPolicyBeResent(policy.Id), Times.AtLeastOnce()); } else { documentResenderRepositoryMock.Verify(x => x.CanPolicyBeResent(policy.Id), Times.Never()); } } } [Test] public void IfNoPoliciesAreFoundThatAreSuitableForDocumentResendingThenGetThePolicyThatHasMostRecentlyExpired() { var unsuitablePolicies = allPolicies.Where(x => x.IsActive() == false && x.IsInTheFuture() == false).OrderBy(x => x.EndDate); var policyWithClosestToEndDateToNow = unsuitablePolicies.ToList().Last(); SetUpDocumentResender(unsuitablePolicies); documentResendService.ResendDocuments(); documentResenderRepositoryMock.Verify(x => x.CanPolicyBeResent(policyWithClosestToEndDateToNow.Id), Times.AtLeastOnce()); foreach (var policy in allPolicies.Where(policy => policy != policyWithClosestToEndDateToNow)) { documentResenderRepositoryMock.Verify(x => x.CanPolicyBeResent(policy.Id), Times.Never()); } } } ```