Refactoring one large test into multiple smaller tests

Today I spent some time refactoring a large test within a client project, splitting it into several smaller tests. The commit removed 169 lines but added 233 lines.

So, why did I do this?

This test is responsible for testing the creation of products and product variants within Drupal Commerce from a custom CSV file and originally had a very generic name - “Should create a product and product variations from an array of data”.

But it did much more than that:

  1. It asserted that there are no initial existing products or product variations.
  2. It ran a product import using some stub data.
  3. It asserted that there are two products, each with two variations.
  4. It asserted that each product has the correct title.
  5. It asserted that each product variation has the correct title.
  6. It asserted that each variation has the correct SKU.
  7. It asserted that each variation has the correct price.
  8. It asserted that each variation has the correct value for 10 product attributes.

All of this was hidden within a single test.

Whilst it was great as the original test name (I usually start with a vague name whilst I’m spiking the first test and until it’s clearer to me what it’s testing and what the correct name is), what I actually want this test to do is to check that the correct number of products and variations are created.

This refactoring task was to split the remaining assertions into their own named tests, after which I had six different tests.

This means that each piece of functionality and related assertions are now contained within their own named tests. I can read the test file and see the expected functionality within the test names rather than everything being grouped and hidden within a single vaguely-named test.

If an assertion fails, I can easily see in which test the failure occurred.

Each test is very simple and only a few lines long - it runs the product import, loads the created variation, and runs the appropriate assertions.

It’ll be much easier to add new functionality to the importer by adding a new separate test rather than continuously adding to the large original one.

Even though there are more lines in the file after the refactoring, most of those are just because of adding the additional test functions. There are only 72 lines of actual test methods, and the reusable steps, such as running the product import as well as custom assertions, are defined as private methods to avoid duplication.

In my opinion, this was a good refactor to do, and now was a good time to do it before we get started on the next phase of the project.