-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add unit test for rlibc #15815
Add unit test for rlibc #15815
Conversation
@@ -28,7 +28,7 @@ | |||
html_root_url = "http://doc.rust-lang.org/master/")] | |||
#![feature(intrinsics)] | |||
|
|||
#![no_std] | |||
//#![no_std] // XXX: this needs to be comment for test macros to be defined..? |
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 will need to stay rather than being commented out. You can follow the same test strategy of crates like libcollections
which have lots of #[cfg(test)]
directives at the top.
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.
Sure, there is one right down below, but it seems to make no effect whrn this one is on :(
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.
Can you explain the errors that you're encountering?
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.
The errors where simply that assert!
(and friends) are not defined. Turns out that it's not enough to add #[cfg(test)] extern crate test;
, which seemed like an obvious way of fixing this...
#[cfg(test)] #[phase(plugin, link)] extern crate std;
Following your advise I checked libcollections/lib.rs
and adding the above for some reason fixes this. This still looks like a little issue in the compiler, don't you think?
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.
Turns out that it's not enough to add #[cfg(test)] extern crate test;,
What is the error? (We'll be able to tell if it's a compiler issue if you tell us the error messages/problems. :) )
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.
The error message without the extra attribute was this:
lib.rs:117:13: 117:22 error: macro undefined: 'assert_eq!'
I am now actually face with something more interesting:
error: type `&'static str` does not implement any method in scope named `len`
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.
Ah, yes, if you're using macros from std
(or core
) you need to import them via a #[phase]
.
For len
, it's a trait method so you need to have the traits in scope. (i.e. add a use std::collections::Collection
in the module with the tests.)
I will add a test for |
r? |
use core::slice::{MutableVector, ImmutableVector}; | ||
use std::c_str::ToCStr; | ||
|
||
#[test] fn work_on_windows() { } // FIXME #10872 needed for a happy windows |
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 removed: #15815 (comment)
Ok, I will squash if everyone is happy :) r? |
#[test] | ||
fn memcpy_and_memcmp_arrays() { | ||
let (src, mut dst) = | ||
([b'X', .. 100], [b'Y', .. 100]); |
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.
Why is there a new line?
chore: fix urls in guide.md This PR fixes two URLs in the guide.md file.
We are pulling in rlibc into Zinc (hackndev/zinc#113), but while reading the code, I though it could do with some unit tests.
So I have added some tests here, but seem like there are a few issues... Please see comments withXXX
and respond if you have any thoughts...