-
Notifications
You must be signed in to change notification settings - Fork 76
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
Fix/ledger tests org update #1343
Fix/ledger tests org update #1343
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
You could make the modifications @willemneal mentioned too, but either way this looks like a good improvement 👍🏻.
I don't feel strongly that you need to do this now, but we normally keep the test and test_fixtures modules under the src directory, not at the top level, so that we are still only linking one binary during tests. While Rust does support outside in testing via the tests/ folder at the top level a problem with it is every test file becomes a separate link target, which means the crate has to be relinked over and over, which slows down running tests. For crates this size it may not matter much.
@leighmcculloch From my understanding if you have So really it's a question about the surface area, should it be external or internal? |
TIL nice, I didn't know about the module/main.rs approach. 🎉 Also yeah this lib is small so probably not a big deal to worry about test organisation too much. The main goal here I think is reducing confusion about what helper functionality is for tests or not, which this does. 👍🏻 |
Great discussion, and thanks for sharing that article @willemneal - super helpful! I also think that if we have |
What
Moves stellar-ledger test-only files into a
test_fixtures
directory.Why
Hopefully, this will make it clearer what files are used just for testing, and which ones are source code.
Known limitations
Once this ticket is complete this test-only code will be pulled out into its own crate.