AssertAll doesn't work in TestCleanup

[Dave]

My general pattern for testing has been to use TestInitialize to set up mocks that are used by all tests, and then in TestCleanup to call AssertAll on each mocked object. However I have just discovered that this doesn't (always?) work. In one of my tests I put an Arrange on one of the mocked objects that I knew would never happen, but the call to AssertAll did not fail the test. However, if I move the AssertAll into the body of the test itself it does indeed fail as it should.
Is this a known bug, or am I doing something wrong? I have used this pattern for a lot of tests which it now seems may not be testing as rigorously as they should.

[Stefan]

Hello Dave,

It is not a bug, it is by design. In TestCleanup you can only assert expectations arranged in TestInitialize. Expectations arranged in a test method can only be asserted in that test method.

Even though it's by design, right now I can't say for sure that it actually makes sense. I will give it some thought, but in the mean time you should move your asserts into the test methods.

Regards,
Stefan
Telerik

 

 

[Dave]

It's also very dangerous because it is completely non-intuitive. It would be completely natural for the test writer (as I did) to factor all the asserts into the TestCleanup function, and unless he/she actually noticed (as I did) that an expectation had been arranged that never actually happened, he/she would never know that the tests were incorrect. I now need to go back through the unit tests for several products, as well as the one I am currently working on, to refactor these cleanup methods. I think it is something that at the very least needs flagging IN BIG LETTERS in the documentation.

[Dave]

I am now hopelessly confused. I have written a small test project - you can infer the contents of Class1 and Class 2, the tests look like this:

[TestClass]
public class Class1Tests
{
    Class2 _mockClass2;
    [TestInitialize]
    public void TestInitialise()
    {
        _mockClass2 = Mock.Create<Class2>(Behavior.Strict);
    }
    [TestCleanup]
    public void TestCleanup()
    {
        _mockClass2.AssertAll();
    }
    [TestMethod]
    public void ShouldFailButDoesnt()
    {
        // Arrange
        _mockClass2.Arrange(x => x.DoesSomething()).Returns(false);

        // No act

        // Assert by TestCleanup, arrangement has not happened.
    }
}

Intuitively the test should pass, however according to what you have just explained I would expect it to fail - HOWEVER it does actually pass!! Can you explain what is going on?

Dave

[Dave]

Oh dear, you see how confused I'm getting. The last sentence should read:
"Intuitively the test should fail, however according to what you have just explained I would expect it to pass- HOWEVER it does actually fail!! Can you explain what is going on?"

[Stefan]

Hello Dave,

Ah, you've found a bug in JustMock. Apparently Mock.AssertAll(o) is not the same as o.AssertAll(), which is a bug. If you replace _mockClass2.Arrange with Mock.Arrange(() => _mockClass2... and _mockClass2.AssertAll(); with Mock.AssertAll(_mockClass2), the behavior that I explained in the previous post will manifest.

I have logged the bug in our bug tracker. I have also logged a bug that the TestCleanup method doesn't retain the mocking context of the test method preceding it. After these bugs are fixed and the fix is made available in one of our internal builds, you will be able to write asserts in the TestCleanup method that work correctly for expectations arranged in the test methods.

As a token of gratitude for your feedback I have given you some Telerik points.

Regards,
Stefan
Telerik

 

 

[Dave]

Thanks Stefan (I'm not quite sure what I do with Telerik points, but I'm sure they'll come in handy :-)),
Will I be, or can I arrange to be, notified when the relevant fixes are released?
Dave

[Kaloyan]

Hi Dave,

We will do our best to let you know when the fixes of the above mentioned bugs are present.

Further, you can check this page inside your Telerik account for more information about the Telerik points. I hope it helps.

Regards,
Kaloyan
Telerik

 

 

[David]

Hi. Please can you tell me if this bug has been fixed. We are now on 2021.1.222.1. Thanks

[Ivo]

Hello David, according to our bug tracker and Q2.2015 Release announcement, the issue was partially fixed. I have tested the provided sample using the latest JustMock version and the behavior is remaining the same. Since the problem is having a relatively big impact on the initial design, I am not able to give you a definitive answer before we make a thorough analysis. I will bring this topic to the team's attention and will come back with the outcome next week.

[Mihail]

Hi David,

I want to jump in and add more info to make the topic clearer. The fix that was made is for the bug reported on 19 Feb 2015 and confirmed by Stefan.

The design still remains the same and calling assert from the TestCleanup method for arrangements created in the TestMethod won't work. Calling assert from the TestCleanup method will work for arrangements created in the TestInitialize method.

If you have any further questions do not hesitate to get back to us.

[David]

Hi guys.

I would just like to say that, while I can understand that this behaviour is intrinsic to the way JustMock works, and could be very difficult to fix, it is nevertheless IMHO an extremely serious weakness in the JustMock framework. It effectively makes it impossible to use [TestCleanup] with any confidence. A [TestCleanup] method is the logical place to put calls to AssertAll() on any mocked objects that are declared as member variables and used by multiple tests. However, such an obvious and intuitive pattern has to be prohibited, as it can in no way be assumed that all the expectations on the mocked object were arranged in the [TestInitialize] method, and any that are not will not be asserted, causing tests to succeed when they should not, as described. We have thousands of tests in hundreds of source files, most of which have a clean-up method of some sort that is to be called at the end of every test. To ensure that this problem is avoided we have commented out the [TestCleanup] decoration, and added a very explicit comment as to why this has been done, exhorting both coders and code reviewers that they must ensure that every test explicitly calls the clean-up method at the end of the test.

I'm really surprised that more people have not fallen foul of this weakness and reported it.

[Mihail]

Hi David,

Thank you for your detailed comment.

I agree with you that it is very convenient to add a general assert all functionality to the [TestCleanup] as it is called after every unit test. Typically the [TestCleanup] is used to clean memory or close streams that have been opened in [TestInitialize] or clean something that will be reused for the next test, like data in a database. Meaning that the purpose of those methods is to prepare the context in which the test will be executed and then clean it. This is the reason why JustMock treats those methods differently from the [TestMethod].

We could refactor the current behavior if a significant number of the clients are using the [TestCleanup] for the assertion of mock objects created in the [TestMethod]. At this point, we don't see such a trend. Until that changes, we will stick to the current implementation.

Once again thank you for your comment and involvement in helping us improve JustMock. I really appreciate it. If you have any other suggestions for improvements please do not hesitate to get back to us.

Best,

Mihail