Should we test private data (static objects in this case) to make sure it maintains its structure?

Question

I had a discussion at work about whether to unit test a private static object we're using as data for a public component.

const data = {
 45: { name: 'John' },
 2: { name: 'Patricia' },
 27: { name: 'Rei' },
}

The list goes on with dozens of items and in the real production example it has many more fields. In general, we shouldn't be testing something like this, it's just a private static object after all! The data is not modified by the component or by any function, it just serves as a static source of truth.

However, we have a data requirement that all names in the list must be unique, a requirement that could change in the future but an actual requirement nonetheless, and we know this requirement because of the nature of the data. Two items with the same name are considered an error in the data of this example.

So let's write a small unit test to make sure all names are unique, something like:

it('should have unique names', () => {
  const dataset = new Set(Object.values(data).map(item => item.name))
  expect(data).to.have.lengthOf(dataset.size)
})

A future maintainer or developer might work on this data in the future, adding or removing list items, and they might not know this requirement.

In this example the fact that names have to be unique is not evident unless you read the test, so this test is acting as documentation for the data, however imagine the real production example with much more data and fields, and more intricate relationships that are, however, easily testable.

I consider the test necessary while as my more senior colleague considered it unnecessary. Their points are quite valid:

• Only test features that you are exposing publicly.
• If someone modifies the data and make it wrong, that's their responsibility, not yours.

My points, I think, are also valid:

• Unless we force the structure via tests, there's no other way to make sure the data won't get messed up in the future.
• How wrong could it be having unit tests that are not testing a public feature but an internal structure?

What do you think and why? Any reference or link to a particular pattern would be appreciated. Thanks!

Answer 1

I wouldn't consider it wrong to check the internal data structure for consistency, especially in a whitebox test scenario. What's the alternative, a documentation comment on the constant declaration? Don't argue with your colleague about necessity, argue about usefulness. Will the test prevent someone (your future self included) from making a mistake? Then it's a good thing, so take up the responsibility to write the test. You can never have too many tests - until they take too long to run, are a burden to maintain, produce inconsistent results etc.

Of course there is a point in testing only the external API of your code, namely that it makes it easier to refactor the internal structures. If you can write a test using a public method that will fail when the internal data structure contains duplicates or is otherwise invalid, then you might want to prefer that approach.

And you're wrong on one account:

Unless we force the structure via tests, there's no other way to make sure the data won't get messed up in the future.

If it's so important not to mess up the structure, why check for that only in a test? Put the check in the actual business logic. Validate the data you're reading when initialising your private data structure. Do it in the component code itself, every time your component is used, even - that's what you care about, right?

Then write a test to verify that your check actually fails when duplicates are present.

Answer 2

A unit test that acts on data by itself and declares the data valid is a dead test because it says nothing about what use the data can be put to.

The proper test is to shove the data into the code that cares about the data's structure and watch for errors. Because when this passes you know why you care.

Unit tests aren't a handy way to externalize validation. They're a handy way to test your validation.

Not that data can't be tested for conformity to a format. That's what WSDLs and schemas are for.

Answer 3

The ultimate purpose of tests is to save time and money by detecting potential errors before they become a problem. Many tests are not worth writing because writing them is more effort than they can be expected to save, in particular if they aren't checking any interesting properties.

But the impact of a potential error depends entirely on the context. Is this a small team that understands the codebase and its implied contracts well, and would the impact of having duplicate names merely be a minor annoyance? Or is this a codebase where people without much understanding will make contributions, and/or would duplicate names take down large parts of the internet?

Only you know what the risk (likelihood and impact) of such errors would be, and consequently whether these tests are worth it. I'm generally a big fan of asserting such internal invariants, but it's not always appropriate. In particular, consider these alternatives:

• Sometimes, a comment that explains the invariants is enough.

  // Data for $fooBar.
  // Note that the names must be unique because $someReason.
  const data = { ... };
  

• Invariants that must always hold during execution aren't necessarily a good fit for tests. Tests are better at checking behaviour or example scenarios.

  Invariants are better checked by asserts within the code so that they are checked during normal execution. If the properties are cheap to check, this is a little bit of overhead but the code becomes more self-documenting (a kind of executable comment, unlike the comment in the previous point) and often much easier to debug because mistakes will be surfaced early.

Regarding some of the mentioned arguments:

• Should only test publicly exposed features?

  On one hand, this is a good heuristic because tests that involve internal implementation details are more brittle – they might break during a later refactoring and slow you down instead of increasing your confidence in your software. Testing internals has the risk that you check only that the software was implemented in a particular manner, not that it works correctly.

  On the other hand, what is public? I think it's perfectly reasonable draw boundaries within your codebase and test those internal modules, regardless of which modular programming feature your programming language actually provides.

  Testing internals, when done right, is not brittle but can also be an insanely effective testing approach. Tests are supposed to be useful, and tests that home in on some critical part of your code can exercise that critical part thoroughly. This is often much easier (and then ultimately more cost-effective) than laboriously writing scenarios that only use the “public” interface. Some useful properties that you might want to check – like the consistency of a purely internal data structure – might be impossible to exercise through the public interface, as in your example.

• If someone modifies the data and make it wrong, that's their responsibility, not yours?

  Blaming others is not a good look. The goal is to have software that works, not to have someone else as a scapegoat.

  Towards that end, it's important to communicate where the sharp edges of your codebase are. Comments, tests, assertions, and type-checks are all tools to communicate our understanding of the code, and prevent others from cutting themselves on the sharp edges of the codebase.

  Computers are there to help, so let computers to the tedious work of checking whether the code looks right. Tests, assertions, and so on are one way to automate this.

• Unless we force the structure via tests, there's no other way to make sure the data won't get messed up in the future?

  I tend to agree, with the caveat that the risk might be so small that a risk mitigation isn't worth it, and that other risk mitigations than tests exists as well.

Personal recommendation: the risks in this case look so small that a mitigation like a test may not be worth it. In particular, it might not be worth expending political capital in your team on this issue. If a mitigation were desired, I'd tend to prefer a comment or an assertion in this scenario. For example, I might create a constructor for that data and have the constructor check for the desired properties. However, I think that there would be absolutely nothing wrong with the kind of test your proposed.