Does my unit test care about too much

I have 2 concerns about my unit test method:

  • Do I test too much in one test method?
  • How can my test method name reflect all test expectations?
  • I asked myself when my method name says: ReturnInvalidModelState , then my 2 other Asserts are not correct. At least concerning the method name...

    [Test]
    public void Create_TemplateAlreadyExists_ReturnInvalidModelState()
    {
        // ARRANGE
        TemplateViewModel templateViewModel = new TemplateViewModel { 
            Name = "MyTest" 
        };
    
        Mock<ITemplateDataProvider> mock1 = new Mock<ITemplateDataProvider>();
        Mock<IMappingEngine> mock2 = new Mock<IMappingEngine>();
    
        TemplateController controller = 
            new TemplateController(mock1.Object, mock2.Object);
        mock1.Setup(m => m.TemplateExists("MyTest")).Returns(true);
        // Set ModelState.IsValid to false
        controller.ModelState.AddModelError("Name", 
                                            "This name already exists.");
    
        // ACT
        ActionResult result = controller.Create(templateViewModel);
    
        // ASSERT
        Assert.IsFalse(controller.ModelState.IsValid);
        Assert.IsInstanceOfType(typeof(PartialViewResult), result);
        Assert.AreEqual(templateViewModel, ((PartialViewResult)result).Model);
    }
    
    [HttpPost]
    public ActionResult Create(TemplateViewModel templateViewModel)
    {
        if (ModelState.IsValid
            && !_templateDataProvider.TemplateExists(templateViewModel.Name))
        {
    
            Template template = 
                Mapper.Map<TemplateViewModel, Template>(templateViewModel);
    
            _templateDataProvider.AddTemplate(template);
            return new JsonNetResult(new { success = true });
        }
        ModelState.AddModelError("Name", "This name already exists.");
        return PartialView(templateViewModel);
    }
    

    Yes, I think that you are testing for too many things.

    Start with renaming your test method. Your method signature should describe action, scenario and expected outcome.

    If I were to rename your method, than I would end up with the following:

    public void Create_DuplicateTemplate_ModelStateIsInvalidAndReturnsPartialViewResultAndPartialViewResultOfTypeTemplateViewModel() 
    { 
    }
    

    Your test is concerned with three things, rather than one. When it fails, you won't know straight away why it has failed.

    Consider re-factoring this into smaller tests and encapsulating some of the arrangement logic so that it can be re-used.

    Edit:

    You made a good point in your comment regarding single test method having a single assertion. I agree with you on that one, as good as it sounds, often it's not sufficient.

    Say I have the following action method:

    [HttpPost]
    public ActionResult Register(NewUserViewModel newUser)
    {
        if (!ModelState.IsValid)
            return View(newUser);
    
        var newUserDTO = Mapper.Map<NewUserViewModel, NewUserDTO>(newUser);
        var userDTO = UserManagementService.RegisterUser(newUserDTO);
    
        var result = Mapper.Map<UserDTO, UserViewModel>(userDTO);
    
        TempData.Add("RegisteredUser", result);
        return RedirectToAction("RegisterSuccess");
    }
    

    I have the following unit test for this method:

            [TestMethod]
            public void Register_HttpPost_ValidViewModel_ReturnsRegisterSuccess()
            {
                // Arrange
                var autoMapperMock = this.mockRepository.DynamicMock<IMapper>();
                var userManagementServiceMock = this.mockRepository.DynamicMock<IUserManagementService>();
    
                var invalidRegistrationViewModel = new NewUserViewModel
                {
                    LastName = "Lastname",
                    FirstName = "Firstname",
                    Username = null
                };
    
                autoMapperMock.Expect(a => a.Map<UserDTO, UserViewModel>(Arg<UserDTO>.Is.Anything)).Repeat.Once().Return(null);
                autoMapperMock.Expect(a => a.Map<NewUserViewModel, NewUserDTO>(Arg<NewUserViewModel>.Is.Anything)).Repeat.Once().Return(null);
                userManagementServiceMock.Expect(s => s.RegisterUser(Arg<NewUserDTO>.Is.Anything)).Repeat.Once();
    
                autoMapperMock.Replay();
    
                var controller = new AccountController
                {
                    Mapper = autoMapperMock,
                    UserManagementService = userManagementServiceMock
                };
    
                this.mockRepository.ReplayAll();
    
                // Act
                var result = (RedirectToRouteResult)controller.Register(invalidRegistrationViewModel);
    
                // Assert
                Assert.IsTrue((string)result.RouteValues["Action"] == "RegisterSuccess");
            }
    

    As you can see, I set up multiple expectations on my mock:

  • I expect AutoMapper to be called twice
  • I expect UserManagementService to be called once
  • At the end of the test I have a single assertion that checks whether user was re-directed to the correct route.

    So where do I check my assertions? I create another method that makes sure that my expectations have been met:

        [TestCleanup]
        public void Cleanup()
        {
            try
            {
                this.mockRepository.VerifyAll();
            }
            finally
            {                
            }
    }
    

    So you are right, I have three assertions instead of one, but I structure my code in such a way so it appears that I have only one assertion.


    I would recomend moving all of the "Arrange" and "Act" code into a Setup() method, and split the rest into three tests. This will make each individual test much easier to read, and let you give each test a name that corresponds better to the actual assert it contains.

    private TemplateViewModel _templateViewModel;
    private ITemplateDataProvider _mock2;
    private IMappingEngine _mock2;
    private TemplateController _controller;
    private ActionResult _result;
    
    [Setup]
    public void Setup(){
        // ARRANGE
        _templateViewModel = new TemplateViewModel { Name = "MyTest" };
    
        _mock1 = new Mock<ITemplateDataProvider>();
        _mock2 = new Mock<IMappingEngine>();
    
        _controller = new TemplateController(_mock1.Object, _mock2.Object);
        _mock1.Setup(m => m.TemplateExists("MyTest")).Returns(true);
    
        // Set ModelState.IsValid to false
        _controller.ModelState.AddModelError("Name", 
                                            "This name already exists.");
    
        _result = controller.Create(_templateViewModel);
    }
    
    
    [Test]
    public void Create_TemplateAlreadyExists_ModelStateIsInvalid()
    {
        Assert.IsFalse(_controller.ModelState.IsValid);
    }
    
    
    [Test]
    public void Create_TemplateAlreadyExists_ResultIsPartialViewResult()
    {
        Assert.IsInstanceOfType(typeof(PartialViewResult), _result);
    }
    
    
    [Test]
    public void Create_TemplateAlreadyExists_ResultModelMatchesTemplateModel()
    {
        Assert.AreEqual(_templateViewModel, ((PartialViewResult)_result).Model);
    }
    
    链接地址: http://www.djcxy.com/p/63308.html

    上一篇: 只有在第二个管道发生故障时才会关闭套管

    下一篇: 我的单元测试是否关心太多