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

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

Closed
ignatenkobrain opened this issue Jul 5, 2017 · 10 comments
Closed

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

ignatenkobrain opened this issue Jul 5, 2017 · 10 comments
Assignees

Comments

@ignatenkobrain
Copy link

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
Copy link
Contributor

behnam commented Jul 5, 2017

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
Copy link
Author

@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
Copy link
Contributor

behnam commented Jul 5, 2017

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
Copy link
Contributor

behnam commented Jul 5, 2017

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

@behnam behnam self-assigned this Jul 5, 2017
@Manishearth
Copy link
Member

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

@Manishearth
Copy link
Member

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

@ignatenkobrain
Copy link
Author

@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
Copy link
Contributor

behnam commented Jul 5, 2017

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

behnam added a commit to behnam/rust-unicode-bidi that referenced this issue Jul 6, 2017
bors-servo pushed a commit that referenced this issue Jul 6, 2017
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 -->
@steveklabnik
Copy link

@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
Copy link
Member

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.

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