chevron-thin-right chevron-thin-left brand cancel-circle search youtube-icon google-plus-icon linkedin-icon facebook-icon twitter-icon toolbox download check linkedin phone twitter-old google-plus facebook profile-male chat calendar profile-male
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 by classala (680 points)

4 Answers

0 votes
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 by lior (13.2k 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 by classala (680 points)
0 votes
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 by tillig (6.7k points)
0 votes
That's great feedback. Thanks. I'll make sure to fix my tests and keep that in mind.
answered by classala (680 points)
...