rillig / gobco

Measure branch coverage of golang tests
62 stars 12 forks source link

Incorrectly flags last switch case #19

Closed vfaronov closed 11 months ago

vfaronov commented 3 years ago

Given the following toy.go:

package toy

import "math/rand"

func Foo() string {
    switch rand.Intn(2) {
    case 0:
        return "zero"
    case 1:
        return "one"
    }
    panic("impossible")
}

and the following toy_test.go:

package toy

import (
    "math/rand"
    "testing"
)

func TestFoo_zero(t *testing.T) {
    rand.Seed(0)
    if s := Foo(); s != "zero" {
        t.Fatalf(`%q != "zero"`, s)
    }
}

func TestFoo_one(t *testing.T) {
    rand.Seed(1)
    if s := Foo(); s != "one" {
        t.Fatalf(`%q != "one"`, s)
    }
}

gobco reports:

Branch coverage: 3/4
toy.go:9:7: condition "rand.Intn(2) == 1" was once true but never false

although both cases are actually covered.

rillig commented 3 years ago

That's a tricky situation, and you need to read the message very carefully.

toy.go:9:7: condition "rand.Intn(2) == 1" was once true but never false

The "once true" refers to TestFoo_one. The "never false" says that the cases rand.Intn(2) == 0 and rand.Intn(2) == 1 are covered, but other values for rand.Intn(2) were never seen. This is not surprising since rand.Intn(2) is guaranteed to only ever return 0 or 1, never a higher number.

In other words, gobco is trying to tell you that you don't need the case 1 and could simply replace it with default:.

If you have an idea how to improve gobco's message to make it easier to understand, please tell me. Otherwise you can just close the issue since gobco is correct here.

vfaronov commented 3 years ago

Ah, right, thank you.

If you have an idea how to improve gobco's message to make it easier to understand, please tell me.

This would help:

toy.go:6:2: "switch rand.Intn(2)" never took the implicit default branch
rillig commented 11 months ago

I thought about the improved diagnostic again, and I'm not going to implement it.