Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testify getting included in binary builds. #1200

Closed
joel-rieke opened this issue Aug 18, 2022 · 6 comments
Closed

Testify getting included in binary builds. #1200

joel-rieke opened this issue Aug 18, 2022 · 6 comments

Comments

@joel-rieke
Copy link

While pulling in the project, I noticed I was getting somewhat larger builds. One thing I did notice is that testify was getting sucked into the builds (which should never happen). This can happen if you don't use the *_test.go convention for all your test files containing testify. I found maybe 5-6? I renamed them in my fork for now because I'm not sure if the way I renamed them would break any of your automated tests.

Simply renaming this files to _test.go equivalents removed testify from the build.

On another note, I wonder if other things could be getting sucked in as well given the thing with testify was happening.

@timsehn
Copy link
Contributor

timsehn commented Aug 18, 2022

You mind sending over a PR with your renamed files?

@joel-rieke
Copy link
Author

Let me check into a few things first. I have other changes that are a part of this branch. If they were accepted, it would be great because I could get rid of my fork.

@fulghum
Copy link
Contributor

fulghum commented Aug 23, 2022

Let us know if you found any more clues. We'd love to get this fixed up and let you get rid of your fork so you can work on tip of main.

@zachmu
Copy link
Member

zachmu commented Oct 12, 2022

I think the reason testify gets pulled in is because we export a set of acceptance tests for integrators (people building a database backend, like Dolt) can use to verify their integration. To my knowledge there is no other way to export test acceptance functionality as a library.

testify may get pulled in by a build process, but it should never actually make it into a binary because the go compiler should mark it as dead code (unreachable by the main method). Are you saying this isn't the case for you? Can you provide a repro?

@joel-rieke
Copy link
Author

Yes, I was definitely seeing testify get sucked into the build. I used "strings" to see that testify was in the final binary. After renaming the test libraries as _test.go, they were correctly excluded. The final binary shrank significantly in size upon doing this.

@zachmu
Copy link
Member

zachmu commented Nov 3, 2022

Thank you for bringing this to our attention.

We did this analysis for dolt, which depends on go-mysql-server, and found that there was only one test file that had this issue, in the server package. Removing that file got rid of the testify inclusion in the dolt binary (after some additional changes on the dolt side, where similar issues were happening).

Fixed in this PR:

#1369

Please try that and let us know if you still see testify getting pulled into your build.

@zachmu zachmu closed this as completed Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants