Unit-testing – How to convince some one that test should do assertion (not assertions) and not the helper methods

coding-standardscoding-styleunit testing

Joined a new employer and came across a new style of writing tests.

@Test()
public testMethodWhichDoesNotDoAnyAssertion() {
    LoginPage loginPage = signUpPage.doLogin("username","password");
    oneMoreCommonMethodCalledHere()
    anotherCommonMethodCalledHere()
}

public void doLogin(String userName, String password) {
     //login here
     Assert.assertTrue("Login Successful")
}

public void oneMoreCommonMethodCalledHere() {
     //Some more operations here.
     Assert.assertTrue("This also succeeded")
}

public void anotherCommonMethodCalledHere() {
     //Some more operations here.
     Assert.assertTrue("Even this succeeded!!! Your code is awesome!!!")

So far I have been doing assertions in tests and not in the methods which are invoked from
test method. The problems I have with approach are multiple –

There are two many assertions happening in one method, though indirectly and it
defeats the idea of one responsibility per test

There are times when I want to do one assertion in my test method while testing for a work flow. And many of the helper methods which would be called would assert things and might even fail which
would hamper work flow test.

Now thing I have heard in favour of this approach –
It is easy for any one to just plug in helper method in a test while not worrying about
the assertions which should be carried out for a scenario, as helper method takes care of it.

Comments?

Best Answer

Below I assume the helper methods you show are test helpers, not part of the production code (this is not clear from your post, as @Martin already noted).

There are two many assertions happening in one method, though indirectly and it defeats the idea of one responsibility per test

I personally don't take the "one assertion per test" rule (or might I call it dogma). I prefer to be pragmatic, and in my experience such a rule would hinder development. My rule is "one scenario per test" - and that single scenario may well require multiple assertions to verify the results.

However, I agree that having assertions intermixed with calls to the code under test is somewhat confusing. I prefer the Arrange-Act-Assert style as it makes my tests clearer - not only for others, but for the future myself too. Nevertheless, I do often put one or more assertions into dedicated helper methods - just in this case, those helper methods are meant only to assert results, not to act, and their name reflects this.

Now, at any rate, being the new guy in the team, you better understand and get used to the already established practices and conventions of the team, in order to become a useful team member (or start looking for a better workplace if you really really really feel you can't take this). After that, you can gradually start infusing your own ideas and thoughts, even criticism, into the thought process of the team. But trying to change team conventions up front, without proving your worth and gaining respect of your colleagues first, is going to be a futile and frustrating experience for you.

There are times when I want to do one assertion in my test method while testing for a work flow. And many of the helper methods which would be called would assert things and might even fail which would hamper work flow test.

Two notes:

  • Why do the assertions fail? My first instinct is that in a good test they shouldn't, and removing the failing assertions may actually hide a real problem.

  • What stops you from refactoring the helper methods? E.g.

    public void doLoginWithoutAssert(String userName, String password) {
     //login here
}

public void doLogin(String userName, String password) {
     doLoginWithoutAssert(userName, password);
     AssertTrue("Login Successful");
}

    This way you can call the assertion-free helper while your colleagues keep calling their original helpers, and everyone's happy.

Related Topic