new test causes old tests to break, am I doing this wrong?

I have a unit tests that test if the first name of a baby is not supplied then don't save the baby, and another that if the first name is supplied then save should be c开发者_运维知识库alled. I did red/green/refactor and it passed. I added new tests that do the same for last name. Now the the tests for first name are failing because the last name is not supplied. I have provided the test code below. I am wondering if I am going about this the wrong way, or am I just expected to correct the broken tests?

In addition the validator interface is necessary because what is valid changes depending on the client using the software, otherwise I would code these checks into the Baby class itself.

Update: based on the few responses I have already recieved, it seems that I am going about this the wrong way. What should I be doing instead so that this situation does not arise?

    [TestMethod]
    public void baby_is_not_saved_if_validation_fails() {
        // arange
        var validator = new Mock<IValidator<Baby>>();
        var output = new ValidationCollection();
        validator.Setup(v => v.IsValid(It.IsAny<Baby>(), out output)).Returns(false);
        var unitOfWork = GetMock();
        // act
        var b = new Baby();
        var svc = new BabyService(validator.Object, unitOfWork.Object);
        svc.AddNewBaby(b);
        // assert
        unitOfWork.Verify(u => u.SaveChanges(), Times.Never());
    }

    [TestMethod]
    public void baby_is_saved_if_validation_passes() {
        // arange
        var validator = new Mock<IValidator<Baby>>();
        var output = new ValidationCollection();
        validator.Setup(v => v.IsValid(It.IsAny<Baby>(), out output)).Returns(true);
        var unitOfWork = GetMock();
        // act
        var b = new Baby();
        var svc = new BabyService(validator.Object, unitOfWork.Object);
        svc.AddNewBaby(b);
        // assert
        unitOfWork.Verify(u => u.SaveChanges(), Times.Once());
    }

    [TestMethod]
    public void if_first_name_is_not_supplied_baby_is_not_added() {
        // arrange
        var validator = new DefaultBabyValidator();
        var unitOfWork = GetMock();
        // act
        var b = new Baby();
        var svc = new BabyService(validator, unitOfWork.Object);
        svc.AddNewBaby(b);
        // assert
        unitOfWork.Verify(u => u.SaveChanges(), Times.Never());
    }

    Mock<IHealthUnitOfWork> GetMock() {
        var uow = new Mock<IHealthUnitOfWork>();
        var dbSet = new Mock<IDbSet<Baby>>();
        dbSet.Setup(db => db.Add(It.IsAny<Baby>())).Returns(new Baby());
        uow.Setup(u => u.SaveChanges()).Verifiable();
        uow.SetupGet(u => u.Babies).Returns(dbSet.Object);

        return uow;
    }

    [TestMethod]
    public void if_first_name_is_supplied_baby_is_added() {
        // arrange
        var validator = new DefaultBabyValidator();
        var unitOfWork = GetMock();
        // act
        var b = new Baby { FirstName = "Charles" };
        var svc = new BabyService(validator, unitOfWork.Object);
        svc.AddNewBaby(b);
        // assert
        unitOfWork.Verify(u => u.SaveChanges(), Times.Once());
    }

    [TestMethod]
    public void if_last_name_is_not_supplied_baby_is_not_added() {
        // arrange
        var validator = new DefaultBabyValidator();
        var unitOfWork = GetMock();
        // act
        var b = new Baby { FirstName = "Charles" };
        var svc = new BabyService(validator, unitOfWork.Object);
        svc.AddNewBaby(b);
        // assert
        unitOfWork.Verify(u => u.SaveChanges(), Times.Never());
    }
}

Your problem is your tests represent contradictory requirements. If the first name is supplied and the last name is not, then if_first_name_is_supplied_baby_is_added() says the baby should be saved, but if_last_name_is_not_supplied_baby_is_not_added() says the baby should not be saved.

The problem is that the original test for first name did not have specs for last name at the time. When your specs change then your tests can/will change. I have refactored my tests so that I do not need to change previous tests as new fields are discovered on the Baby class. I now have one test method that asserts the true condition for a valid baby. The required field tests now check the following:

• if required field is not valid
  • IsValid must return false
  • the expected exception must exist in the returned collection (AggregateException.InnerExceptions)
• if required field is valid
  • only check that the expected exception does not exist

Now I only have to update one test (the all fields are valid test) when new fields are discovered.

[TestMethod] // only update this one as new fields are discovered
public void when_baby_is_valid_validation_returns_true() {
    var validator = new DefaultBabyValidator();

    // valid baby goes here
    var baby = new Baby();

    AggregateException fail;
    Assert.IsTrue(validator.IsValid(baby, out fail));
}

[TestMethod]
public void validation_returns_exception_if_first_name_is_null() {
    var validator = new DefaultBabyValidator();
    AggregateException results;
    var isValid = validator.IsValid(new Baby(), out results);
    var expected = results.InnerExceptions
                    .OfType<ArgumentException>()
                    .SingleOrDefault(ex => ex.ParamName == "FirstName");
    Assert.IsFalse(isValid); // should always return false if this condition is met
    Assert.IsNotNull(expected); // should also contain the expected exception
}

[TestMethod]
public void validation_does_not_return_exception_if_first_name_is_valid() {
    var validator = new DefaultBabyValidator();
    AggregateException results;
    validator.IsValid(new Baby { FirstName = "Charles" }, out results);
    var expected = results.InnerExceptions
        .OfType<ArgumentException>()
        .SingleOrDefault(ex => ex.ParamName == "FirstName");
    Assert.IsNull(expected); // exception should not exsist. 
                             // We don't care if it returns true.
                             // There is only one case where IsValid should
                             // return true.
}