6

For reasons lost in the mists of time, your project has a code coverage standard. Unfortunately, some new code has gone in which takes the coverage below this figure. John Doe is duly assigned to write more tests (since Jane Doe is off snowboarding). John is unfamiliar with the code and drags the coverage over the line by calling various constructors and methods with mocks. None of these adhere to the accepted given... when... then... pattern and so the purpose of the new test code is hard to ascertain.

Leaving aside the rights and wrongs of:

a) A code coverage figure

b) The tests not being written at the same time as the code

Do the new tests constitute technical debt? What should be done about them (if anything)?

Laiv
  • 14,283
  • 1
  • 31
  • 69
Robbie Dee
  • 9,717
  • 2
  • 23
  • 53
  • 5
    I don't think they constitute a code smell. They constitute a process smell. Particularly, the "arbitrary code coverage figure smell", which however you explicitly excluded from your question. – Jörg W Mittag Sep 14 '17 at 21:47
  • 2
    "Code smell" is not a meaningful term; you should stop using it. – Robert Harvey Sep 14 '17 at 21:48
  • @RobertHarvey Agreed, it is a pretty broad church - but technical debt doesn't really fit either. Any ideas? – Robbie Dee Sep 14 '17 at 21:52
  • related (possibly a duplicate): [Is test coverage an adequate measure of code quality?](https://softwareengineering.stackexchange.com/q/192/31260) – gnat Sep 14 '17 at 21:52
  • 5
    It *is* technical debt. Anything you do in a half-assed way to meet a shipping deadline (that you're only going to have to fix later) is technical debt. – Robert Harvey Sep 14 '17 at 21:54
  • @gnat The figure itself is somewhat tangential to the question - it could equally stand without a recognised figure but I accept the wider point. – Robbie Dee Sep 14 '17 at 21:54
  • @RobertHarvey Sounds reasonable, I shall edit accordingly. – Robbie Dee Sep 14 '17 at 21:58
  • Is it technical debt or just garbage? Or is that the same thing? I don't like new terms that are longer than the previous terms. – Frank Hileman Sep 14 '17 at 23:18
  • @FrankHileman Technical debt is as vague and meaningless as code smell, but on balance, I think technical debt is a better fit here. – Robbie Dee Sep 15 '17 at 08:29

4 Answers4

13

Yes, I'd say these are technical debt. Test code is not different than the code being tested in that it must also be maintained and understood. The fact that you don't understand the point of the testing code is a maintenance problem as it doesn't help you understand the code which is being tested; a large component of what makes testing useful to begin with. This makes things harder than not having any tests at all.

The coverage requirement is also pointing out a non-code issue, which is a developer that doesn't care enough about doing his job properly. Not understanding the code is a poor excuse to write useless tests to just to satisfy the build. That should be addressed as well. If this is typical behavior for John, I'd be looking to get him off the same team as me.

The metric being including in the build is likely an attempt to get people like John to actually write the tests they are supposed to. Its much harder to justify not writing tests if the result is a broken build, which is typically taken pretty seriously by software teams.

Andy
  • 2,003
  • 16
  • 22
  • Re your comment: *This makes things harder than not having any tests at all* - are you saying it would be better if they were removed? – Robbie Dee Sep 15 '17 at 17:41
  • 2
    @RobbieDee Well the ideal would be meaningful tests, but if that can't be done for some reason, yes, tests which are meaningless will cause confusion about what the class should be doing... it'd be better not to have them at all. – Andy Sep 15 '17 at 19:38
1

Do the new tests constitute technical debt?

Yes. Eventually, these horrible tests will need to be modified, probably by Jane when she returns from snowboarding. At which time, she will struggle with understanding the tests.

What should be done about them (if anything)?

If the horrible tests are acceptable for the short term, keep them. If not, fix them. When Jane returns from snowboarding, she should delete the new tests all together and replace them with acceptable tests (assuming that she is the best person to do so).

leo1
  • 49
  • 3
1

Your technical debt was introduced at the point where functionality was implemented without appropriate test coverage.

Unless you are literally better off without those unit tests, (in which case delete them now) I'd say that its not appropriate to state that the tests themselves constitute technical debt. Rather the tests are a sub-par attempt to address the absence of appropriate test coverage.

Justin
  • 1,728
  • 10
  • 16
-1

The new tests don't constitute a technical debt simply because they were written to satisfy a code coverage metric. The reason for writing them is now in the past. Particular problems with the tests may be tech debt, but without knowing more about those problems we can't say whether there are problems best remedied by deleting the tests.

What should be done about the tests now depends on what the nature of the tests is and what affects they have, rather than the John's motivation for writing them. Keep them if they are useful, delete or modify them if they are harmful.

You don't say whether or not these are assertion free tests. Assertion free tests have little value, although they can catch some bugs particularly if the code is an interpreted language where you don't have a build stage to catch syntax and type errors. If they are assertion free it might be worth keeping them but adding assertions.

bdsl
  • 2,365
  • 1
  • 14
  • 14