Welcome to Typemock Answers. Here you can ask and receive answers from other community members. And if you liked or disliked an answer or thread: react with an up- or downvote Enjoy!

Is this the best way to write this test?

0 votes
Hi there,

I wrote a test here and I'd like to know if this is the best way to do it. Essentially I have the following class:

    public class MyClass
    {
        public Guid Id { get; set; }
        
        public IDbCommand GetCommand()
        {
            IDbCommand command = this.MakeCommand();
            command.CommandText = this.BuildQuery();

            IDbDataParameter parameter = command.CreateParameter();
            parameter.ParameterName = "@PK";
            parameter.Value = this.Id;

            command.Parameters.Add(parameter);

            return command;
        }

        protected virtual IDbCommand MakeCommand()
        {
            return null;  // will use a factory class here eventually...
        }


        public string BuildQuery()
        {
            return "some query...";
        }
    }


I want to test the GetCommand method, for which i have the following expectations:

* It must return a valid instance of an IDbCommand object;
* Its CommandText property has to be filled with whatever the BuildQuery method returns;
* It must have one "@PK" parameter;
* the "@PK" parameter's value must be MyClass' Id property.

At first I wrote the test using Reflective mocks, like so:

        [TestMethod]
        [VerifyMocks]
        public void Test2()
        {
            string EXPECTED_QUERY = "SELECT PK_Contact, FirstName, LastName FROM Contacts " +
                                  "WHERE FK_Company = @PK " +
                                  "ORDER BY FirstName, LastName";

            string EXPECTED_PK_PARAMETER_NAME = "@PK";

            Guid EXPECTED_PK_VALUE = new Guid("8FB32696-BDB2-4CD7-880B-12A412D0C67A");

            MockObject<IDbDataParameter> parameterMockBehavior = MockManager.MockObject<IDbDataParameter>();
            parameterMockBehavior.ExpectSet("ParameterName").Args(EXPECTED_PK_PARAMETER_NAME);
            parameterMockBehavior.ExpectSet("Value").Args(EXPECTED_PK_VALUE);
            IDbDataParameter parameterMock = parameterMockBehavior.Object;

            MockObject<IDataParameterCollection> parameterCollectionMockBehavior = 
                MockManager.MockObject<IDataParameterCollection>();
            parameterCollectionMockBehavior.ExpectAndReturn("Add", 1).Args(parameterMock);

            MockObject<IDbCommand> commandMockBehavior = MockManager.MockObject<IDbCommand>();
            commandMockBehavior.ExpectAndReturn("CreateParameter", parameterMock);
            commandMockBehavior.ExpectGet("Parameters", parameterCollectionMockBehavior.Object);
            commandMockBehavior.ExpectSet("CommandText").Args(EXPECTED_QUERY);

            Mock myClassMockBehavior = MockManager.MockAll<MyClass>();
            myClassMockBehavior.ExpectAndReturn("MakeCommand", commandMockBehavior.Object);
            myClassMockBehavior.ExpectAndReturn("BuildQuery", EXPECTED_QUERY);

            MyClass myClass = new MyClass();
            myClass.Id = EXPECTED_PK_VALUE;
            IDbCommand command = myClass.GetCommand();
            Assert.IsNotNull(command);
        }



Then, I refactored the test so to use Natural mocks:

        [TestMethod]
        [VerifyMocks]
        public void Test1()
        {
            string EXPECTED_QUERY = "SELECT PK_Contact, FirstName, LastName FROM Contacts " +
                                  "WHERE FK_Company = @PK " +
                                  "ORDER BY FirstName, LastName";

            string EXPECTED_PK_PARAMETER_NAME = "@PK";

            Guid EXPECTED_PK_VALUE = new Guid("8FB32696-BDB2-4CD7-880B-12A412D0C67A");

            MockObject<IDbDataParameter> parameterMockBehavior = MockManager.MockObject<IDbDataParameter>();
            IDbDataParameter parameterMock = parameterMockBehavior.Object;

            MockObject<IDbCommand> commandMockBehavior = MockManager.MockObject<IDbCommand>();
            IDbCommand commandMock = commandMockBehavior.Object;

            using (RecordExpectations recorder = new RecordExpectations())
            {
                parameterMock.ParameterName = EXPECTED_PK_PARAMETER_NAME;
                parameterMock.Value = EXPECTED_PK_VALUE;

                commandMock.CommandText = EXPECTED_QUERY;

                commandMock.CreateParameter();
                recorder.Return(parameterMock);

                commandMock.Parameters.Add(parameterMock);
                recorder.Return(1);
            }

            Mock myClassMockBehavior = MockManager.MockAll<MyClass>();
            myClassMockBehavior.ExpectAndReturn("MakeCommand", commandMock);
            myClassMockBehavior.ExpectAndReturn("BuildQuery", EXPECTED_QUERY);

            MyClass myClass = new MyClass&
asked Feb 7, 2008 by classala (200 points)

4 Answers

+1 vote
Hi,

Actually you are doing quite good here.
There is nothing significant i would have done differently. (maybe just using simple mock creation instead of the MockAll usage)
Since you are mocking a combination of private and public methods you need to combine the usage natural with reflective and that is OK.

Another approach that you could have taken, which might rersult in a simpler test, would be to check the returning IDbCommand instead of mocking all the internal calls on the command class.
I think that doing so might shorten your test and make it more readable.
answered Feb 7, 2008 by lior (18,340 points)
0 votes
Hi Lior,

thanks for the quick reply.

When I first started writing the test, I actually wrote the code that'd just test the return of GetCommand, and I'd put assertions checking for properties on the object (i.e., CommandText, Parameters, etc.).

However, the return is based on the IDbCommand interface, and since I'm not instantiating a concrete implemention of that interface (didn't want bring in dependencies on, say SqlDbCommand, or didn't want to create a dummy class of my own), the only I though I could write this was by mocking anything related to IDbCommand, IDbDataParameter, etc.
answered Feb 7, 2008 by classala (200 points)
+1 vote
There is nothing significant i would have done differently. (maybe just using simple mock creation instead of the MockAll usage)


I second the notion that you should not use MockAll unless you absolutely have to. We've run into problems where you have test fixtures with hundreds of tests and helper methods to set up mocking of complex dependencies and whenever we run into any problems it's generally because someone used MockAll when they only needed to mock a single object.

A good rule of thumb is to only mock exactly what you need - no more, no less.
answered Feb 7, 2008 by tillig (6,430 points)
0 votes
That's great feedback. Thanks. I'll make sure to fix my tests and keep that in mind.
answered Feb 7, 2008 by classala (200 points)
...