-
Notifications
You must be signed in to change notification settings - Fork 33
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
cargo test doesn't work from crates.io #43
Comments
Yeah, we don't package the test data files into the library crate. I don't know what's the common best-practice here. Do most crates package test data files in? And more generally, @ignatenkobrain, why package from crates.io and not the original repo? |
@behnam most of packages just don't use include/exclude, so everything ends up in crates.io... And speaking of crates.io, it is basically what users get when using cargo. There are a lot of complications around sub-crates and such, so we decided to just use crates.io. |
Actually, only resources (.rs, etc) used during a normal build get pulled in automatically. I'm not sure if there's an option to enable test config for packaging to get the test data file auto-detected. So, I believe we have to add it to Cargo.toml explicitly. I'm surprised that the
Yeah, I hear you. It can get very complicated. I always wondered if there's a case for doing anything besides "building" from crates.io. Now there is. |
Yeah, you're right @ignatenkobrain. I forgot that we're excluding |
Common practice is to exclude large data files, this is why include/exclude exist. |
TLDR you are not expected to be able to run tests off crates.io. |
@Manishearth I know, I'm fine with having just doc-tests, but if you include tests to archive and they do not work... well ;) |
Cool! So I'll exclude all non-build assets, which would also reduce the package size a lot. |
Upgrade to Unicode 10.0.0 and improve packaging * Upgrade to Unicode 10.0.0 is straightforward. One of the unit tests (for code points falling back to default value in Unicode 9.0.0) needed update, so I also updated the structure of the unit test. * To address <#43>, I have excluded all non-`src` directories, as they all depend on the text files in the `data` dir, which we don't want to include. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/unicode-bidi/44) <!-- Reviewable:end -->
Where are you getting this from? Cargobomb, for example, relies on this. |
Pretty sure I've seen discussions about this before. Not saying you should never publish tests to crates, but if there is a challenge in publishing a test to crates it's fine to remove it. Which is what we've done here.
Cargobomb is relatively new; and relies on some crates' tests working, not all of them. Cargobomb doesn't expect all crates to have working tests. |
When packaging for Fedora I noticed that
cargo test
doesn't work due toSo either it makes sense to include them or exclude tests ;)
The text was updated successfully, but these errors were encountered: