I have also seen high coverage that didn’t actually test much of anything. Code with side effects, none of which tested, just more or less that the method ran without a runtime exception.
Coverage is a good tool but a bad metric.
Thats emphatically not true if the tests are very low level unit tests that tightly couple to implementation. Theyre a greater impediment to refactoring than 0 tests.
Ive worked on a couple of projects like this and seen the same crystallization effect as the OP where people were visibly more afraid of touching unit tested code.
These type of tests A) never catch a bug but B) will always fail when you change the code for any reason and C) theyre intimidating to maintain.
That means if you refactor the code you usually need to test it just as much manually on top of making the useless tests go green, following whatever dumb pattern they were written with. That increases the workload of changes on that chunk of code by 10-40%.
Strictly speaking these types of tests are better off deleted but mid level/junior developers and even many seniors who ought to know better will balk at it due to loss aversion.
Ive noticed that this is also the type of bullshit test ChatGPT will write when cajoled so I anticipate seeing much more of this in the years to come.
If tests are low quality, they somehow managed to pass the code review. So, I would have more wider questions to the project, knowing this.
Adjusting tests require 10% during refactor? Yes, it is fine (getting some uncovered rare regression bug in prod is not fine).
Manual testing after refactor? No, it is not fine (and manual testing should be automated before the refactor. With end-to-end/integration testing. In the ideal world).
Sadly, it all depends on managers, many projects are just "Just doo it", so nobody cares about keeping the tests sane.
Oh yea, flaky, weird, non-documented tests would increase load by 40% to the new devs and that is true evil.
The answer to this is almost always a lack of skill or that the team was focused on delivery and were willing to neglect test quality in pursuit of that goal.
Either way it's a common enough situation that can't just be handwaved away. Even if you've fired the team members responsible for that code the code doesnt just go away.
They spend time designing the architecture for whatever they are implementing but when it comes to writing tests, all of that stuff goes out the window and people just do whatever takes the least amount of time.
If you start treating tests as any other code, it should get easier to change as your implementation changes, and you'll get a lot more value out of automated testing.
For one, this has resulted in a curious structure. I have a 5 or 6 fixtures with representative sample datasets used in production and these get extended as new features are introduced. If some new addition breaks everything, we'll know immediately, as all tests use the same fixtures and it's all just one place. And it's also instructive during design. These fixtures give you 3-4 situations to think through when adding something new.
And the tests are pretty. Most of them have been boiled down into one test_client request and a comparison with a dataset, just 2-3 logical lines. This makes is easy to see if tests are still correct, necessary. Or to just talk about if they make sense overall, even with less technical or involved people.
No, please, don't treat test code as any other production code. Test code can have duplication throughout if it helps readability, test code that relies on abstractions, utility methods, etc. are hard to read, which makes the test cases themselves hard to understand.
Test code should be easy to read a case, understand what the test case is preparing for (I like to add comments marking given/when/then sections to make it very clear), what is being called, with what parameters, and what exactly the expected result is.
I had my fair share of interactions with test code trying to be smart, test code abstracting away the assertions (into utility methods that themselves call other utilities), test code trying to avoid duplication, and causing headaches and requiring troubleshooting of test code... That is not the reason for test code to exist, it's not to be clean and concise. It's test, you need to know what it is doing, and very quickly assess what failed when a case fails.
Please, don't overengineer test code, it should be elegant but there's no need to write it as you would write production code, write it as documentation.
I'm not arguing for making the test cases harder to read, it's very important to be able to see input/output/expected output as always, but it's also important that you're able to change the test code confidently, and duplication actively works against that.
I guess the "truth" or "optimal state" is somewhere in-between. Not 100% duplication but also not 0% abstractions.
Exactly, I've seen way too much test code that abstracted away a fundamental piece needed for understanding the test setup, which made it much harder to reason about when tests failed. Jumping through the test to the utility used to avoid duplication is another layer of cognitive load, I'm already trying to troubleshoot something, I don't need more things to keep track of while I'm in that process.
Rewriting test code is exactly the moment you take to abstract some of it away, when it's been proven that the pattern repeats and is annoying to rewrite. The usual rule of 3 of software engineering.
> it's also important that you're able to change the test code confidently, and duplication actively works against that.
I disagree with this point a little, if the tests are well written (even if verbose) it should be easy to reconfigure/rewrite them, even with dozens of test cases, it might take a bit more time to rewrite those than just changing a supplier/utility method but it has helped me a lot to actually read the tests. I've been burnt too many times by someone abstracting away the actual call (or a part of it) to the logic under test into an utility to "avoid duplication", that call is the whole point of the test case. One should be able to understand every test case easily so rewriting to accommodate the change made should be straightforward, if it's not then the test case itself is telling there's a code smell, either in the test or in what is under test.
The more egregious to me is when tests are a mess of setups not following a simple, straightforward structure. It becomes hard to reason and change over time when the inevitability of growing code happens. Duplication or not won't be the main issue, the structure itself will.
I will abstract away some complex setup/adjacent code that isn't under test but anything that the test cases might interact with, and fail because of it I do prefer to be extremely explicit and duplicate code instead of trying to make my tests look tidy and neat.