deadcode: unreachable func which is exclusively used in tests

[Chris Burkert]

Hey there,

I have a package "test", which is only used in tests and which contains one function: https://go.dev/play/p/Ne1xGXsQNmy

This leads to a finding when running deadcode:

$ deadcode ./...

test/testg.go:8:6: unreachable func: SkipIfIntegration

I tried several things:

- renaming the package to "test_test"

- renaming the file to "test_test.go"

I think https://github.com/golang/go/issues/31135 and https://github.com/golang/go/issues/39565 address the same topic.

It seems that in *_test.go files you cannot import from other *_test.go files in other packages.

My workaround: I run deadcode -test ./... but then I may miss unused code, which may have an old test.

Is there a better way?

PS: I want to follow https://www.konradreiche.com/blog/how-to-separate-integration-tests-in-go/

thanks

Chris

[Axel Wagner]

On Tue, 19 Nov 2024 at 15:25, Chris Burkert <burker...@gmail.com> wrote:

Hey there,

I have a package "test", which is only used in tests and which contains one function: https://go.dev/play/p/Ne1xGXsQNmy

This leads to a finding when running deadcode:

$ deadcode ./...

test/testg.go:8:6: unreachable func: SkipIfIntegration

To me, that seems like an issue with the deadcode linter. In particular, I don't understand why it would ever complain about an exported function. That is almost definitionally not "dead".

I tried several things:

- renaming the package to "test_test"

- renaming the file to "test_test.go"

I think https://github.com/golang/go/issues/31135 and https://github.com/golang/go/issues/39565 address the same topic.

It seems that in *_test.go files you cannot import from other *_test.go files in other packages.

My workaround: I run deadcode -test ./... but then I may miss unused code, which may have an old test.

Is there a better way?

Maybe you can add something like `var _ = SkipIfIntegration` to your package? i.e. trick the linter to stop it from complaining?

 

PS: I want to follow https://www.konradreiche.com/blog/how-to-separate-integration-tests-in-go/

thanks

Chris

--
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 visit https://groups.google.com/d/msgid/golang-nuts/CALWqRZpziVRJ_9ifRXNEzhGk8negv7xbjwZod24sSbcuvt9KQQ%40mail.gmail.com.

[Chris Burkert]

Hi Axel,

Maybe you can add something like `var _ = SkipIfIntegration` to your package? i.e. trick the linter to stop it from complaining? 

I already tried this, but it did not help.

I even added the following, which made things worse:

func init() {

_ = SkipIfIntegration

}

$ deadcode ./...

test/test.go:8:6: unreachable func: init#1

test/test.go:12:6: unreachable func: SkipIfIntegration

[Axel Wagner]

You could also call it from a test within your test package.

[Axel Wagner]

Hm, sorry, no, come to think of it, that wouldn't help either.

So, I don't know. Apart from "don't use that linter" or "ignore that complaint", I'm out of ideas.

[Axel Wagner]

okay, sorry, I should think a bit more.

What if you do something like

func init() {

    if false { SkiptIfIntegration(nil) }

}

i.e. actually insert a call, but put it into an unconditionally false branch, to make sure it doesn't use its nil argument? If that doesn't help either, you can further obfuscate the false into something that is false, but that can only be determined at runtime to be false (like `var x uint64; if x+1 == 0 { SkipIfIntegration(nil) }`).

[Chris Burkert]

Well, the init() function in the test package leads to an additional complaint:

$ deadcode ./...

test/test.go:8:6: unreachable func: init#1

test/test.go:14:6: unreachable func: SkipIfIntegration

It works it I import the test package in my main package and fake call it there: if false { test.SkipIfIntegration(nil) }

But this feels so wrong :-(

I also read about the -filter flag of deadcode, but I couldn't figure out a regex, which would exclude the test package.

[Dan Kortschak]

From the executable's docs[1],

> In any case, just because a function is reported as dead does not
> mean it is unconditionally safe to delete it. For example, a dead
> function may be referenced by another dead function, and a dead
> method may be required to satisfy an interface that is never called.
> Some judgement is required.

I'd say that this is not a linter that should be used in CI due to
false positives and the facts that CI cannot exercise judgement.

[1]https://pkg.go.dev/golang.org/x/tools/internal/cmd/deadcode