-
Notifications
You must be signed in to change notification settings - Fork 717
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
Make cargo test
work
#95
Conversation
Hm this is usually what I see when I link to the wrong clang, not sure why that would be happening on travis...
|
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 great to me. The error you're getting is because travis still doesn't have llvm 3.9, so we're stuck with 3.8 until they whitelist it :/
@@ -87,7 +87,7 @@ impl Annotations { | |||
/// | |||
/// the generated code would look something like: | |||
/// | |||
/// ```rust | |||
/// ```rust,ignore |
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.
this should be ```c++`.
- git add -A | ||
- git diff @ | ||
- git diff-index --quiet HEAD | ||
- cargo test |
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.
you'll need to run cargo test --features llvm_stable
, otherwise you're compiling with llvm 3.9 support, which can't be done for now (see #70).
test: regen-tests | ||
@echo > /dev/null | ||
test: | ||
cargo test |
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.
same here, I think we want to stick with llvm_stable
for now.
@@ -40,10 +40,7 @@ before_script: | |||
|
|||
script: | |||
- cargo build --verbose --features llvm_stable | |||
- make test | |||
- git add -A | |||
- git diff @ |
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.
I'd also keep the diff
and diff-index
checks, since they're harmless, but avoid potential errors.
After rebasing on master, I get this failure when running the tests:
|
This was due to mixing up I've addressed review comments, rebased on master, and force pushed. Hopefully the travis ci will come back green and we can merge this. |
CI failed again because of different test expectations for |
Green, yay! @bors-servo: r+ |
📌 Commit 7cadabb has been approved by |
⚡ Test exempted - status |
Fixes #51.
r? @emilio