search

servo / unicode-bidi

Implementation of the Unicode Bidirection Algorithm in Rust
Other
78 stars 33 forks source link

cargo test doesn't work from crates.io #43

Closed ignatenkobrain closed 7 years ago

ignatenkobrain commented 7 years ago

When packaging for Fedora I noticed that cargo test doesn't work due to

error: couldn't read tests/data/BidiTest.txt: No such file or directory (os error 2)

So either it makes sense to include them or exclude tests ;)

behnam commented 7 years ago

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?

ignatenkobrain commented 7 years ago

@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.

behnam commented 7 years ago

most of packages just don't use include/exclude, so everything ends up in 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 tests/*.rs files are already included, though. I need to figure out the details.

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.

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.

behnam commented 7 years ago

Yeah, you're right @ignatenkobrain. I forgot that we're excluding *.txt. I'm on it.

Manishearth commented 7 years ago

Common practice is to exclude large data files, this is why include/exclude exist.

Manishearth commented 7 years ago

TLDR you are not expected to be able to run tests off crates.io.

ignatenkobrain commented 7 years ago

@Manishearth I know, I'm fine with having just doc-tests, but if you include tests to archive and they do not work... well ;)

behnam commented 7 years ago

Cool! So I'll exclude all non-build assets, which would also reduce the package size a lot.

steveklabnik commented 7 years ago

@manishearth

TLDR you are not expected to be able to run tests off crates.io.

Where are you getting this from? Cargobomb, for example, relies on this.

Manishearth commented 7 years ago

Where are you getting this from?

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, for example, relies on this.

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.