I remember once reviewing a colleagues code, he had a method something like this:

public int GetFinishedCount()
{
  int count = GetCount();
  ResetCount();
  return count;
}

I wasn't very impressed but I didn't really have the words at the time to describe what we often call "code smell". We use the phrase in accounting and law as well as engineering and it simply means "something is not quite right but it might take me some time to work out why!".

In this case, the answer is simple. The method naming is implying that this is a sort of immutable method: you are going to get something and that is always harmless. It is also idempotent, meaning you could call it twice (if you lost the value for example) and as long as nothing else has changed, you would get the same result.

But it isn't. It has side-effects that are not obvious and which could easily be exposed to the higher level or simply to change the name of the method to something more explicit GetAndResetFinishedCount().

Anyway, fast-forward to today and a worrying "fix" that was made in ASP.Net Core for the WebApplicationFactory, a very helpful utility that can create an in-memory web server for testing applications.

The basic cause for the "fix" was that if you have not set your asp environment (to select appsettings for example) in Visual Studio, it helpfully sets it to "Development", whether you like it or not. In most cases, that is no biggy (although it does slightly fly in the face of obvious functionality since it will override your local OS env variables). Anyway, when calling the same app from the WebApplicationFactory, it does not use this VS fudgery so somebody using it would need to explicitly set the env to "Development", otherwise the behaviour via VS would be different (by default) from that in test.

There are a couple of ways this could have been handled and possibly the original nasty bit was launchsettings.json in VS, but anyway, we could have:

  1. Automatically add a launchsettings.json for testing too. We need to document that when running locally, you have to do some work to not be "Development"
  2. Maybe use the VS launchsettings for the factory (if possible). Same as above
  3. Remove environment variables from launchsettings.json and simply set the default value of env to "Development" but always override it from your OS.
  4. Require the env parameter to be set when creating a factory so it is obvious you get nothing by default (it could even provide a parameter default but ideally it needs to allow auto-injection from environment).
But nope. Perhaps because of time or complexity or whatever, MS went for the smelliest change. They will force WebApplicationFactory to use "Development", regardless of what the OS environment is! This is hidden, of course, so the only time you realise it is broken is if you are lucky (or unlucky) enough for some wrong app settings to be used and the system falls over or the tests fail (a worse scenario would be screwing up a live database because it is using the wrong connection strings).

Well. Everyone makes mistakes right? But then someone raises a more than fair objection that when running his tests on the CI server (as I also discovered) it doesn't work because it is using appsettings.Development.json, even though I am on "Build". It is not obvious what is happening or why but I then find the issue. The response should have been, "whoops, that's a bit of a screw-up actually" but instead was along the lines of both "why would you ever need to do that?" and then "it works as designed and can be worked around so won't be fixed".

You could also read this as, "we broke it but since you can work around the break, we're leaving it alone". It is sad because it basically says, "we are not agile enough to make another change" or "we don't believe that quality is a main goal", or maybe, "we know what we're doing and why so get over it"

None of this is good, along with the myriad of other bugs, MS doesn't need better people or processes and it certainly doesn't need any more money, it needs a better culture. A culture where code smells are not made in the first-place, where complaints are not dismissed, where bugs that might seem minor are actually treated with the magnitude of problems they are likely to cause globally and not simply a massive backlog of stuff that is likely to be fobbed off. Ultimately, most of us would be happier without C# 8 and instead with more effort on making things work consistently well or being re-written/re-factored if they don't. Visual Studio is terribly buggy and allows its extensions to make it even worse.

Please Microsoft, sort it out. Understand your customers and also the way these issues and their responses are perceived!