Google Testing Blog

Testing on the Toilet: Only Verify Relevant Method Arguments

Tuesday, June 26, 2018

This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.
By Dillon Bly
What makes this test fragile?

@Test public void displayGreeting_showSpecialGreetingOnNewYearsDay() {
  fakeClock.setTime(NEW_YEARS_DAY);
  fakeUser.setName("Fake User”);
  userGreeter.displayGreeting();
  // The test will fail if userGreeter.displayGreeting() didn’t call  
  // mockUserPrompter.updatePrompt() with these exact arguments.
  verify(mockUserPrompter).updatePrompt(
      "Hi Fake User! Happy New Year!", TitleBar.of("2018-01-01"), PromptStyle.NORMAL);
}

The test specifies exact values for all arguments to mockUserPrompter. These arguments may need to be updated when the code under test is changed, even if the changes are unrelated to the behavior being tested. For example, if additional text is added to TitleBar, every test in the codebase that specifies this argument will need to be updated.
In addition, verifying too many arguments makes it difficult to understand what behavior is being tested since it’s not obvious which arguments are important to the test and which are irrelevant.
Instead, only verify arguments that affect the correctness of the specific behavior being tested. You can use argument matchers (e.g., any() and contains() in Mockito) to ignore arguments that don't need to be verified:

@Test public void displayGreeting_showSpecialGreetingOnNewYearsDay() {
  fakeClock.setTime(NEW_YEARS_DAY);
  userGreeter.displayGreeting();
  verify(mockUserPrompter).updatePrompt(contains("Happy New Year!"), any(), any()));
}

Arguments ignored in one test can be verified in other tests. Following this pattern allows us to verify only one behavior per test, which makes tests more readable and more resilient to change. For example, here is a separate test that we might write:

@Test public void displayGreeting_renderUserName() {
  fakeUser.setName(“Fake User”);
  userGreeter.displayGreeting();
  // Focus on the argument relevant to showing the user's name.
  verify(mockUserPrompter).updatePrompt(contains("Hi Fake User!"), any(), any());
}

No comments

Testing on the Toilet: Keep Tests Focused

Monday, June 11, 2018

This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.
By Ben Yu
What scenario does the following code test?

TEST_F(BankAccountTest, WithdrawFromAccount) {
  Transaction transaction = account_.Deposit(Usd(5));
  clock_.AdvanceTime(MIN_TIME_TO_SETTLE);
  account_.Settle(transaction);


  EXPECT_THAT(account_.Withdraw(Usd(5)), IsOk());
  EXPECT_THAT(account_.Withdraw(Usd(1)), IsRejected());
  account_.SetOverdraftLimit(Usd(1));
  EXPECT_THAT(account_.Withdraw(Usd(1)), IsOk());
}

Translated to English: “(1) I had $5 and was able to withdraw $5; (2) then got rejected when overdrawing $1; (3) but if I enable overdraft with a $1 limit, I can withdraw $1.” If that sounds a little hard to track, it is: it is testing three scenarios, not one.

A better approach is to exercise each scenario in its own test:

TEST_F(BankAccountTest, CanWithdrawWithinBalance) {
  DepositAndSettle(Usd(5));  // Common setup code is extracted into a helper method.
  EXPECT_THAT(account_.Withdraw(Usd(5)), IsOk());
}
TEST_F(BankAccountTest, CannotOverdraw) {
  DepositAndSettle(Usd(5));
  EXPECT_THAT(account_.Withdraw(Usd(6)), IsRejected());
}
TEST_F(BankAccountTest, CanOverdrawUpToOverdraftLimit) {
  DepositAndSettle(Usd(5));
  account_.SetOverdraftLimit(Usd(1));
  EXPECT_THAT(account_.Withdraw(Usd(6)), IsOk());
}

Writing tests this way provides many benefits:

Logic is easier to understand because there is less code to read in each test method.

Setup code in each test is simpler because it only needs to serve a single scenario.

Side effects of one scenario will not accidentally invalidate or mask a later scenario’s assumptions. 

If a scenario in one test fails, other scenarios will still run since they are unaffected by the failure.

Test names clearly describe each scenario, which makes it easier to learn which scenarios exist.

One sign that you might be testing more than one scenario: after asserting the output of one call to the system under test, the test makes another call to the system under test.

While a scenario for a unit test often consists of a single call to the system under test, its scope can be larger for integration and end-to-end tests. For example, a test that a web UI can send email might open the inbox, click the compose button, write some text, and press the send button.

6 comments

Code Health: Understanding Code In Review

Tuesday, May 01, 2018

This is another post in our Code Health series. A version of this post originally appeared in Google bathrooms worldwide as a Google Testing on the Toilet episode. You can download a printer-friendly version to display in your office.
By Max Kanat-Alexander

It's easy to assume that a developer who sends you some code for review is smarter than you'll ever be, and that's why you don't understand their code.
But in reality, if code is hard to understand, it's probably too complex. If you're familiar with the programming language being used, reading healthy code should be almost as easy as reading a book in your native language.
Pretend a developer sends you this block of Python to be reviewed:

def IsOkay(n):
  f = False
  for i in range(2, n):
    if n % i == 0:
      f = True
  return not f

Don't spend more than a few seconds trying to understand it. Simply add a code review comment saying, "It's hard for me to understand this piece of code," or be more specific, and say, "Please use more descriptive names here."
The developer then clarifies the code and sends it to you for review again:

def IsPrime(n):
  for divisor in range(2, n / 2):
    if n % divisor == 0:
      return False

  return True

Now we can read it pretty easily, which is a benefit in itself.
Often, just asking a developer to clarify a piece of code will result in fundamental improvements. In this case, the developer noticed possible performance improvements since the code was easier to read—the function now returns earlier when the number isn't prime, and the loop only goes to n/2 instead of n.
However, now that we can easily understand this code, we can see many problems with it. For example, it has strange behavior with 0 and 1, and there are other problems, too. But most importantly, it is now apparent that this entire function should be removed and be replaced with a preexisting function for detecting if a number is prime. Clarifying the code helped both the developer and reviewer.
In summary, don't waste time reviewing code that is hard to understand, just ask for it to be clarified. In fact, such review comments are one of the most useful and important tools a code reviewer has!

1 comment

Testing on the Toilet: Cleanly Create Test Data

Tuesday, February 20, 2018

This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.
By Ben Yu
Helper methods make it easier to create test data. But they can become difficult to read over time as you need more variations of the test data to satisfy constantly evolving requirements from new tests:

// This helper method starts with just a single parameter:
Company company = newCompany(PUBLIC);


// But soon it acquires more and more parameters.
// Conditionals creep into the newCompany() method body to handle the nulls,
// and the method calls become hard to read due to the long parameter lists:
Company small = newCompany(2, 2, null, PUBLIC);
Company privatelyOwned = newCompany(null, null, null, PRIVATE);
Company bankrupt = newCompany(null, null, PAST_DATE, PUBLIC);

// Or a new method is added each time a test needs a different combination of fields:
Company small = newCompanyWithEmployeesAndBoardMembers(2, 2, PUBLIC);
Company privatelyOwned = newCompanyWithType(PRIVATE);
Company bankrupt = newCompanyWithBankruptcyDate(PAST_DATE, PUBLIC);

Instead, use the test data builder pattern: create a helper method that returns a partially-built object (e.g., a Builder in languages such as Java, or a mutable object) whose state can be overridden in tests. The helper method initializes logically-required fields to reasonable defaults, so each test can specify only fields relevant to the case being tested:

Company small = newCompany().setEmployees(2).setBoardMembers(2).build();
Company privatelyOwned = newCompany().setType(PRIVATE).build();
Company bankrupt = newCompany().setBankruptcyDate(PAST_DATE).build();
Company arbitraryCompany = newCompany().build();

// Zero parameters makes this method reusable for different variations of Company.
// It also doesn’t need conditionals to ignore parameters that aren’t set (e.g. null
// values) since a test can simply not set a field if it doesn’t care about it.
private static Company.Builder newCompany() {
  return Company.newBuilder().setType(PUBLIC).setEmployees(100); // Set required fields
}

Also note that tests should never rely on default values that are specified by a helper method since that forces readers to read the helper method’s implementation details in order to understand the test.

// This test needs a public company, so explicitly set it.
// It also needs a company with no board members, so explicitly clear it.
Company publicNoBoardMembers = newCompany().setType(PUBLIC).clearBoardMembers().build();

You can learn more about this topic at http://www.natpryce.com/articles/000714.html

7 comments

Testing on the Toilet: Only Verify State-Changing Method Calls

Monday, December 11, 2017

@Test public void addPermissionToDatabase() {
  new UserAuthorizer(mockUserService, mockPermissionDb).grantPermission(USER, READ_ACCESS);

  // The test will fail if any of these methods is not called.
  verify(mockUserService).isUserActive(USER);
  verify(mockPermissionDb).getPermissions(USER);
  verify(mockPermissionDb).isValidPermission(READ_ACCESS);
  verify(mockPermissionDb).addPermission(USER, READ_ACCESS);
}

The answer is that the calls to verify the non-state-changing methods can be removed.
Method calls on another object fall into one of two categories:

State-changing: methods that have side effects and change the world outside the code under test, e.g., sendEmail(), saveRecord(), logAccess().

Non-state-changing: methods that return information about the world outside the code under test and don't modify anything, e.g., getUser(), findResults(), readFile().

You should usually avoid verifying that non-state-changing methods are called:

It is often redundant: a method call that doesn't change the state of the world is meaningless on its own. The code under test will use the return value of the method call to do other work that you can assert.
It makes tests brittle: tests need to be updated whenever method calls change. For example, if a test is expecting mockUserService.isUserActive(USER) to be called, it would fail if the code under test is modified to call user.isActive() instead.
It makes tests less readable: the additional assertions in the test make it more difficult to determine which method calls actually affect the state of the world.
It gives a false sense of security: just because the code under test called a method does not mean the code under test did the right thing with the method’s return value.

Instead of verifying that they are called, use non-state-changing methods to simulate different conditions in tests, e.g., when(mockUserService.isUserActive(USER)).thenReturn(false). Then write assertions for the return value of the code under test, or verify state-changing method calls. Verifying non-state-changing method calls may be useful if there is no other output you can assert. For example, if your code should be caching an RPC result, you can verify that the method that makes the RPC is called only once. With the unnecessary verifications removed, the test looks like this:

@Test public void addPermissionToDatabase() {
  new UserAuthorizer(mockUserService, mockPermissionDb).grantPermission(USER, READ_ACCESS);

  // Verify only the state-changing method.
  verify(mockPermissionDb).addPermission(USER, READ_ACCESS);
}

That’s much simpler! But remember that instead of using a mock to verify that a method was called, it would be even better to use a real or fake object to actually execute the method and check that it works properly. For example, the above test could use a fake database to check that the permission exists in the database rather than just verifying that addPermission() was called.

You can learn more about this topic in the book Growing Object-Oriented Software, Guided by Tests. Note that the book uses the terms “command” and “query” instead of “state-changing” and “non-state-changing”.

3 comments

Testing Blog

Testing on the Toilet: Only Verify Relevant Method Arguments

Testing on the Toilet: Keep Tests Focused

Code Health: Understanding Code In Review

Testing on the Toilet: Cleanly Create Test Data

Testing on the Toilet: Only Verify State-Changing Method Calls

Labels

Archive

Feed