Redbox left PII on decommissioned machines

Perhaps a better example is a real world example I ran into just this week.

I found out that our unit test suite would only pass when run under elevated credentials. Our internal developer tooling had been running under semi-privileged credentials for years, and was the usual way of triggering a full unit test suite run, so no-one really noticed that it didn't work when run at a lower elevation.

When run from a lower privilege, a unit test was failing because it was failing to write to a registry key. I first double checked that I wasn't accidentally triggering integration tests, or that the test should be tagged integration.

But no, we had simply failed to abstract away our registry writes within that service. Of course no unit test should be writing to the real registry, but this settings manager was just being new'ed up as a concrete class, and there was no interface for it, and so it was just naively making registry edits.

This settings class wrote directly to the windows registry as it's data-store wasn't noticed as an issue for years, because all the times it had previously been run, it had been under credentials which could access that registry key.

And yes, there are different ways we could have mocked it, but favouring a concrete class meant this registry edit was happening unnoticed across all our unit test runs. And I suspect this might have been behind some of the dreaded flaky test syndrome, "I just tweaked a CSS file, why did my PR build fail?". Because 99% of times it was fast enough that it didn't cause issues, but with just the right concurrency of test execution, and you'd have a problem that wouldn't show up in any meaningful error message, just a failed test "for no reason".

Why shouldn't unit tests read real-world files? Because that introduces brittleness, and an inherent link between tests. If you want fast and easy to parallelize tests they need to have no real-world effects.

A test suite which is effectively pure can be executed orders of magnitude more quickly, and more reliably, than one which depends on:

  - Files

  - DateTime.Now (Anywhere in your code. A DateTimeFactory which you can mock might sound ridiculous, but it's perhaps the best thing you can do for your code if your current code / tests run on real dateTimes. Even for production code, having a DateTimeFactory can be really helpful for relieving some timing issues. )

  - Databases ( This is more "obvious", but still needs to be said! )

And so on. A unit test suite should boil down to essentially pure statements. Given inputs A,B,C, when applying functions f, g, h, then we expect results h(g(f(A,B,C))) to be X.

This can also be the difference between a test taking As a final point, you're usually not wanting to "test the code that uses a config file", you want to test code which you don't care if it uses a config file.

The "Code that uses a config file" should be your Configurator class. What you actually want to test is some service which actually produces useful output, and that contains business logic.

Yes, "separation of concerns" can be taken too far, but having something else responsible for the manner in which your service is configured so that your service can just take in a business-domain relevant typed settings object is helpful.

As I've said elsewhere, config is actually a terrible example, because it's essentially a solved problem, MS released System.Configuration.ConfigurationManager ( https://www.nuget.org/packages/system.configuration.configur... ), and you should probably use it.

If you're not using that, you ought to have a good excuse. "Legacy" is the usual one of course.

A fist class function is just an interface. Just ask the venerable master Qc Na.

> To me having a separate class to deserialize each kind of XML document to its respective object

You mean a separate function? Or a separated method in some class? (Given that C# can't do top level functions...)

Or else, you may want a type parametrized function that uses reflection to populate an object of any class you want. That would depend on how many different kinds of file you have, but the C# standard library has this function so you just use it.

The thing that should be tested is whatever you are handing to a client app to use. If it is an interface, then test the interface, not the file behind it. If the client will get the file, then test that the file is correct.

So in this case, is this entire file being handed to clients to do what they will with it? Does that make sense as an interface.

If you are building an app, and you want other parts of the app, or clients of the app, to use this data, does it make sense to just hand over the entire file as the interface.

Basically :

Programmer 1: "Hey man, show me your data interface so I can build a quick lookup on something"

Programmer 2: "here is the whole damn file, do whatever you want and leave me alone. Figure it out yourself, and no I'm not going to give you a schema and I'll change it whenever I want".

I'm 'kinda curious to know which movie is this.