Code Coverage: More Is Less?

I started working on some unit tests for the PhidgetException class.  Following the DRY principles, I re-wrote the constructor logic from this

    internal PhidgetException(int code) : base(string.Concat(new object[] { "PhidgetException ", code, " (", GetErrorDesc(code), ")" }))

    {

        this.desc = "Uninitialized Error";

        this.errType = this.errorCodeToErrorType(code);

        this.errCode = code;

        this.desc = GetErrorDesc(code);

    }

    public PhidgetException(string message, ErrorType type) : base("PhidgetException " + type.ToString() + " (" + message + ")")

    {

        this.desc = "Uninitialized Error";

        this.errType = type;

        this.errCode = 0;

        this.desc = message;

    }

    public PhidgetException(string message, int code) : base(string.Concat(new object[] { "PhidgetException ", code, " (", message, ")" }))

    {

        this.desc = "Uninitialized Error";

        this.errType = this.errorCodeToErrorType(code);

        this.errCode = code;

        this.desc = message;

    }

to this:

        public PhidgetException(string message, ErrorType errorType, int errorCode)
            : base(string.Concat(new object[] { "PhidgetException ", errorCode, " (", GetErrorDesc(errorCode), ")" }))
        {
            if (errorType == ErrorType.PHIDGET_ERR_OK && errorCode != 0)
            {
                this._errorCode = errorCode;
                this._errorType = this.ErrorCodeToErrorType(errorCode);
            } 

            if (errorType != ErrorType.PHIDGET_ERR_OK && errorCode == 0)
            {
                this._errorCode = ErrorTypeToErrorCode(errorType);
                this._errorType = errorType;

            }


            if (errorType != ErrorType.PHIDGET_ERR_OK && errorCode != 0)
            {
                int expectedErrorCode = ErrorTypeToErrorCode(errorType);
                if (expectedErrorCode != errorCode)
                {
                    throw new ArgumentException("Error Code and Error Type Do Not Match");
                }
                else 
                {
                    this._errorCode = errorCode;
                    this._errorType = errorType;
                }
            }            

            if (message == string.Empty)
                this._errorDescription = GetErrorDesc(errorCode);
            else
                this._errorDescription = message;
        }

 

        public PhidgetException(string message, ErrorType errorType)
            : this(message, errorType, 0)
        {
        }

 

        public PhidgetException(string message, int errorCode)
            : this(message, ErrorType.PHIDGET_ERR_OK, errorCode)
        {
        }

 Honestly, the 1^st constructor should be marked as private so that you can’t ever get to that ArgumentException, but that is another day’s refactoring.

I then layered some Unit Tests on top of the different constructors to see if my logic worked:

        [TestMethod()]
        public void PhidgetExceptionConstructorTest_Code1_ReturnsPHIDGET_ERR_NOTFOUND()
        {
            PhidgetException phidgetException = new PhidgetException("Test Message", 1);
            ErrorType expected = ErrorType.PHIDGET_ERR_NOTFOUND;
            ErrorType actual = phidgetException.Type;
            Assert.AreEqual(expected, actual);
        }


        [TestMethod()]
        [ExpectedException(typeof(ArgumentException))]
        public void PhidgetExceptionConstructorTest_PHIDGET_ERR_NOMEMORY_ErrorCode3_ThrowsException()
        {
            ErrorType errorType = ErrorType.PHIDGET_ERR_NOMEMORY;
            PhidgetException phidgetException = new PhidgetException("Test Message", errorType,3);
            Assert.Fail("Should Never Get Here");
        }



        [TestMethod()]
        public void PhidgetExceptionConstructorTest_PHIDGET_ERR_NOMEMORY_ErrorCode2_ReturnsErrorCode2AndPhidgetNoMemory()
        {
            ErrorType errorType = ErrorType.PHIDGET_ERR_NOMEMORY;
            int errorCode = 2;
            string message = "Test Message";
            PhidgetException phidgetException = new PhidgetException(message, errorType, errorCode);
            Assert.AreEqual(errorCode, phidgetException.Code);
            Assert.AreEqual(errorType, phidgetException.Type);

        }

So far so good. 

I then did a code coverage analysis of my project.  Note that the Code Coverage screens in VS2010 are not the most intuitive, you need to press that cleverly-hidden “Configure” label (why no Button VS team?):

47% – WTF?

Drilling down, I saw that I did not check the internal method.  Easy enough to fix, I added

[assembly: InternalsVisibleToAttribute("Com.Tff.Phidget.Tests")]

 to my PhidgetException code file (above the namespace, please), wrote a couple of covering Unit Tests

        [TestMethod()]
        public void GetErrorDescriptionTest_ErrorCode2_ReturnsValidString()
        {
            string notExpected = string.Empty;
            string actual = PhidgetException.GetErrorDesc(2);
            Assert.AreNotEqual(notExpected, actual);
        }

 
        [TestMethod()]
        [ExpectedException(typeof(PhidgetException))]
        public void GetErrorDescriptionTest_ErrorCodeNeg1_ThrowsException()
        {
            string notExpected = string.Empty;
            string actual = PhidgetException.GetErrorDesc(-1);
            Assert.Fail("Should Never Get Here");
        } 

 And now got

50%????!!!!  What is going on here?  I have no other methods to cover.  Drilling down, I re-learned the lesson of paying too much attention to Code Coverage.  Check out the code coverage of the 2 lookup methods.

These lookup methods look like this:

 You can see that unless I write 30 covering unit tests for each possible error code and inspect the return value, I will have low code coverage.  A good lesson re-learned.