Code coverage in error cases when compared to other languages

[Charles Hathaway]

Hi all,

I'm looking for a good study/quantitative measure of how well-written Go code looks compared to other languages, such as Java, when it comes to test coverage. In particular, how handling errors may reduce the percentage of code covered by tests in Go relative to other languages.

For example, in this code snippet:

func Foo() error {

  // do some stuff that actually adds value
  if err := somelib.Bar(); err != nil {

    // triggering the error case in Bar is hard, i.e. requires simulating network troubles

    // or causing a file write to fail, but we don't do anything with result besides

    // return it. Testing it by adding an interface or wrapper isn't worth the effort

    // and the only impact is really reported test coverage.

    return err

  }

  // do more stuff
  return nil

}

In Java, you would just add 'throws SomeException' to your method declaration. The effect is that we have one line in the Go code which is not easily covered by a test, whereas Java does not report that untested case because the return path is not visible in the code.

The result is that otherwise equivalent code, we will report different code coverage values, with Go being slightly lower. I'm just looking for something written on that topic that can give us a notion of how much of a difference we might expect.

Thanks,

  Charles

[Axel Wagner]

Hi,

I don't think there is as much of a difference as you think.

You seem to be considering the `throws SomeException` to not impact coverage - but that's not true. It's code you add for error handling and that code is not hit, unless your test actually triggers that exception - just as the code you add for error handling in Go isn't hit. So if you don't count `throws SomeException` as code to be covered in java, you also shouldn't count `if err i= nil { return err }` as code to be covered in Go. So the semantic difference really comes down to a single `throws SomeException` line being able to cover *multiple* branches with the same exception type. It's a difference, but it should be small in practice.

But really, I think what this comes down to is that line-coverage - or, what's actually measured and then projected down to lines, "instruction-coverage" - just isn't a super meaningful measure in this context. More interesting would be branch- or path-coverage - and that would be exactly the same in both cases. Every point where a `SomeException` *could* be thrown would branch off a separate path, just as every `if err != nil` in your Go code. And in both languages they are covered iff you write a test-case that triggers that error condition.

So… I'm sorry that I can't really provide a quantitative, meaningful answer to your question. I don't know what relative difference there would be in line-coverage for Go vs. Java in a case like that. But your question sounds as if you would like to use line-coverage as a metric (maybe even in CI *shudder*) to determine whether you tested enough. And the point I'm trying to make is that I think that goal is fallacious :) If you need a coverage-metric, use branch- or path-coverage, which won't have that difference. But really, coverage reports are IMO most useful if inspected manually, to choose where to invest further tests. As a metric, it just is too unreliable.

https://en.wikipedia.org/wiki/Goodhart%27s_law

 

Thanks,

  Charles

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/6b48ed73-1963-482e-aff0-b91f3aa6a2aen%40googlegroups.com.

[Szczepan Faber]

Good question and useful discussion!

What is Go community guidance on the _value_ of unit testing the `if err i= nil { return err }` idiom?

To make the question a little more precise, let's consider the code snippet in the first email in this thread. Let's assume that I already have coverage for Foo() function happy path. Does it make sense to increase the code complexity (adding mocks) in order to achieve a higher test coverage (covering 'return err' line)? Would that additional coverage be useful given that 'return err' has no complexity and Go has the compiler/linter?

Full disclosure: I'm biased to avoid unit testing those idioms by default. However, I'm very curious what's the community guidance, any documents/links I can read, any reference codebases?

Thank you all!

[Thomas Bushnell BSG]

IMO:

To give these things names, you have:

func Foo(...) ..., error {

  do things here...

  err := makeASubroutineCall(...)

  if err != nil {

    return ..., err

  }

  do more things

}

And we suppose that makeASubroutineCall is already will tested and you know it returns errors correctly. What is the point of a separate test of Foo that makes sure that it returns the errors in the same cases?

Suppose Foo had a bug, and *stopped* returning that error. Wouldn't you want to know? Especially since in many cases, not returning that error can cause the rest to subtly seem right (perhaps makeASubroutineCall syncs something to disk or kicks off some secondary process....)...

Someone comes along, see, and your function becomes this:

func Foo(...) ..., error {

  do things here...

  if moonIsGreenCheese() {

    err := makeASubroutineCall(...)

    if err == nil && someCondition {

      err = someOtherError

    }

    if err != nil {

      return ..., err

    }

  }

  do more things

}

And then another person comes along, and now it's this:

func Foo(...) ..., error {

  do things here...

  if moonIsGreenCheese() {

    err := makeASubroutineCall(...)

    if err == nil && someCondition {

      err = someOtherError

    }

  } else if moonIsRedCheese() {

    err := makeDifferentCall(...)

    if err == nil && someOtherCondition {

      err = yetAnotherError

    }

  }

  if err != nil {

    return ..., err

  }

  do more things

}

Do you see the bug? Wouldn't you be glad if you had a test hanging around to catch it?

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/1acd3558-93ca-494c-b639-af6f694a1fcfn%40googlegroups.com.

[Charles Hathaway]

Hey all,

Thomas, I agree that the code you provide would benefit from unit tests, but I would like to focus my question on the very common case which simply throws the error without additional edge cases, such as the example given in the first email.

Looking at the feedback given so far, I think we have:

- So far, we don't have quantitative literature on how this pattern impacts code coverage in Go versus other languages (Axel)

- Reconsider using code coverage as the primary metric for code quality (Axel)

  - Understood, but it is still a common metric and we should be ready to answer questions about how it relates to Go.

- Does it make sense to increase code complexity to do this additional unit testing (Szczepan)

  - Yes, I think that is the essence of my problem; for what it's worth, this seems to be an easy way to mock it, but feels non-idiomatic

  - Also, apologies, but I can't share actual source code or point to examples, but they should be very common

Thanks all for the feedback!

  Charles

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/YC6A4DQEyl8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CA%2BYjuxtWuNbSePd855tiJRT7fZF3%3DoFz1ve2PnP8St%3DkbteCEg%40mail.gmail.com.

[Uli Kunitz]

The case of somelib.Bar() can be solved by replacing it with an interface. You can than inject a Bar function that always returns an error.

Otherwise I would recommend to use fuzzers to increase code coverage. They will do a much better job than you do. For byte sequence interfaces that don't involve checksums or are encrypted they are extremely good in finding edge cases. I have no experience for business logic, but I would give a JSON encoding of the input parameters a shot.

Kind regards,

Ulrich

[Thomas Bushnell BSG]

On Thu, Feb 4, 2021 at 4:35 PM Charles Hathaway <chat...@google.com> wrote:

Hey all,

Thomas, I agree that the code you provide would benefit from unit tests, but I would like to focus my question on the very common case which simply throws the error without additional edge cases, such as the example given in the first email.

How do you know the code throws an error without testing to see that it does, in fact, throw the error?