Export test-specific functions without go:linkname

[adombeck]

Using go:linkname is poorly supported by tooling, for example it breaks code navigation because IDEs don't support going to the function's implementation.

To still make it clear that those functions are only for use in tests, and to group test-specific functions together in code completion lists, we are now prefixing them with "Z_ForTests". The "Z_" should ensure that they are listed at the end (although that doesn't work for Intellij unfortunately).

UDENG-5449

[adombeck]

Blocked by #641

[adombeck]

In Intellij, the Z_ prefix does not provide any value, because the code completion popup doesn't order completions alphabetically. @didrocks could you check if it has the desired effect in your editor?

[didrocks]

In Intellij, the Z_ prefix does not provide any value, because the code completion popup doesn't order completions alphabetically. @didrocks could you check if it has the desired effect in your editor?

Yes, vscode is using ascii ordering for it.

[3v1n0]

Mhmh, I agree go:linkname isn't the nicest thing, but by adding actually linked functions, don't we end up adding test-only code to the final binaries?

[adombeck]

Mhmh, I agree go:linkname isn't the nicest thing, but by adding actually linked functions, don't we end up adding test-only code to the final binaries?

No, the Go compiler removes functions from the binary which are not used: https://stackoverflow.com/a/48688904/2804197

For methods it's apparently harder to detect, and in fact the two methods which were part of this PR were included in the authd binary. I pushed a fixup commit which turns those into functions and now none of the Z_ForTests functions is included in the binary:

go build -o authd ./cmd/authd && go tool nm authd | grep Z_ForTests

[3v1n0]

go build -o authd ./cmd/authd && go tool nm authd | grep Z_ForTests

Maybe we can add this check to a CI job too?

[adombeck]

Maybe we can add this check to a CI job too?

We could, but do we care that much about not adding test-only code to the binaries? What's the problem with it? It just increases the binary size by a few kilobyte.

[adombeck]

rebased on main and resolved merge conflicts

testing

[3v1n0]

Not totally a fan of having these kind of test-only functions, as I'd like that the language would provide some nicer way for us to use them without having to explicitly including the code (not sure if using instead some kind of go shared library could help?).

However, not blocking this, indeed it has good side effects for us developers...

As further work, it would be nice if we could plan couple of things:

• Add CI job checking symbols aren't there
• See if we can protect us even more by using go:build: feature and include these at least some functions only if a tag is defined (making them no-op otherwise).

[adombeck]

Not totally a fan of having these kind of test-only functions, as I'd like that the language would provide some nicer way for us to use them without having to explicitly including the code (not sure if using instead some kind of go shared library could help?).

Didier also didn't like having test-only functions in non-test code. We discussed possible alternatives but to the best of our knowledge, Go doesn't provide any other way than this go:linkname workaround to make internal functions of another package usable in test code (and only test code) of other packages. I find that the go:linkname workaround very annoying when navigating through the code, so we discussed other options.

This PR implements the compromise we found, which is to prefix it with Z_ForTests so that it's clear from the name that they are test-only and so that they are listed at the end in the code completions pop-up, which was important for Didier. I find that prefix not aesthetically pleasing and would have preferred a ForTests suffix, but that's the compromise we found.

However, not blocking this, indeed it has good side effects for us developers...

As further work, it would be nice if we could plan couple of things:

* Add CI job checking symbols aren't there

I won't push for that because I don't see why that's important, but I won't stand in the way either, so feel free to create a ticket for it.

* See if we can protect us even more by using `go:build:` feature and include these at least some functions only if a tag is defined (making them no-op otherwise).

We already considered that. If I remember correctly, Didier pointed to a related open issue in the go stdlib, but I can't find it.