3

I have two methods , OpenCertificateStore and FindCertificateBySubjectName and implemented them as following:

 public void OpenCertificateStore()
        {
            if (_certificateStore == default(X509Store))
                _certificateStore = new X509Store(StoreLocation.CurrentUser);

            _certificateStore.Open(OpenFlags.ReadOnly | OpenFlags.IncludeArchived);
        }

        public X509Certificate2Collection FindCertificateBySubjectName(string certificateSubjectName)
        {
            X509Certificate2Collection certificates = new X509Certificate2Collection();
            if (_certificateStore != default(X509Store))
            {
                certificates = _certificateStore.Certificates.Find(X509FindType.FindBySubjectName, certificateSubjectName, true);
            }

            return certificates;
        }

I have my unit test as below:

[TestClass]
    public class MyHealthTests
    {
        private Mock<Logger> _logger;
        private Mock<MYCertificateManager> _certManager;

        [TestInitialize]
        public void Initialize()
        {
             _logger = new Mock<Logger>();
             _certManager = new Mock<MYCertificateManager>();
        }

        [TestMethod]
        public void PassName_FindCertiFicatebyName_ShouldReturnValid()
        {


            MyCertificateHelper myCertHelper = new MyCertificateHelper(_logger.Object,_certManager.Object);

            myCertHelper.OpenCertificateStore();
            var certNameCollection = myCertHelper.FindCertificateBySubjectName("Valid Cert Name");
            Assert.IsNotNull(certNameCollection);
            Assert.IsTrue(certNameCollection.Count > 0);
        }
    }

Which works fine , but it would be lot better if I can find a way to mock myCertHelper.

If I do mok them , it returns null as it's not querying actual certificate store.

Simsons
  • 12,295
  • 42
  • 153
  • 269

1 Answers1

3

How do you mock MyCertificateHelper?

You don't.

Doing so would have no benefit. If you did, then all of the classes in your test would be mocked out and you would no longer actually be testing any of your code. At that point, you might as well delete the test. It wouldn't do anything but cost you money to maintain it.


  • Prefixing everything with My is useless. Worse than useless, it's noisy and distracting. Drop it.
  • I don't like the temporal coupling in your design. I don't like needing to call methods like Open or Init. It's easy to forget to call it or call it too many times. It's better if the constructor puts the class into a usable state.
  • It's nice that you're injecting and kicking the logger, but I find injecting loggers to be a code smell. I find it's much nicer to have my classes raise events and have the logger listen for those events. It removes the need to mock the logger all the time and provides nice hooks for other code to leverage. This kind of event driven design ends up in code that is much more open to extension, but closed for modification.
RubberDuck
  • 11,933
  • 4
  • 50
  • 95
  • 'My ' is replaced With Actual class names as I do not want to expose the class name in public. It has a prefix to client name :) – Simsons Jul 03 '17 at 23:22
  • 1
    That's not much better @Simsons. That's what namespaces are for. – RubberDuck Jul 03 '17 at 23:23
  • I'm sceptical about two and three. There are a some objects that you first configure and then use Open or something else to make them usable (see SqlConnection). This pattern has a name but I forgot it... Raising events for logging limits the possibilities of using many advanced scenarios. Injecting even several services is not a problem with such helpers as Autofac and you don't have to implement raising the event in every class. – t3chb0t Jul 04 '17 at 04:13
  • It depends on usecase @t3chb0t, but I've dealt with enough terribly noisy logs that I really prefer to log at as high a level as possible. As for the "Open" thing, Ado is what taught me to hate that pattern and you'll find McConnel arguing against it in Code Complete. – RubberDuck Jul 04 '17 at 10:03