Skip to content

fix some tests for Android #693

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 24, 2017
Merged

fix some tests for Android #693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f72287e
fix some tests for Android
ndusart Jul 20, 2017
a546ef9
use std::env::temp_dir() to retrieve the temp directory in test_mkstemp
ndusart Jul 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/sys/test_pthread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ use nix::sys::pthread::*;
#[test]
fn test_pthread_self() {
let tid = pthread_self();
assert!(tid > 0);
assert!(tid != 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the man page for pthread_self says we should only use pthread_equals for comparing pthreads. If anything we shouldn't even check this value as it's non-portable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering that when I read that test.

So should we remove it ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, remove it, but leave it as a separate commit with a description of why it was removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 still isn't valid here, we should really fix this at some point, but this is fine for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I just remove the test leaving the test_pthread.rs file for a future test to be inserted later or should I completely remove the file ?

Concerning the validity of the test. Maybe we have to wait pthread_create is defined in nix, so that we could make a test where we spin a thread and test that pthread_self returns different pthread_t in the different threads.
And we shouldn't even test for an "invalid" return value as pthread_self is not meant to fail, man page says it always succeed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this test as-is is fine.

}
4 changes: 2 additions & 2 deletions test/test_net.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use nix::net::if_::*;

#[cfg(target_os = "linux")]
#[cfg(any(target_os = "android", target_os = "linux"))]
const LOOPBACK: &'static [u8] = b"lo";

#[cfg(not(target_os = "linux"))]
#[cfg(not(any(target_os = "android", target_os = "linux")))]
const LOOPBACK: &'static [u8] = b"lo0";

#[test]
Expand Down
19 changes: 10 additions & 9 deletions test/test_unistd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use nix::unistd::*;
use nix::unistd::ForkResult::*;
use nix::sys::wait::*;
use nix::sys::stat;
use std::iter;
use std::{env, iter};
use std::ffi::CString;
use std::fs::File;
use std::io::Write;
Expand Down Expand Up @@ -64,22 +64,23 @@ fn test_wait() {

#[test]
fn test_mkstemp() {
let result = mkstemp("/tmp/nix_tempfile.XXXXXX");
let mut path = env::temp_dir();
path.push("nix_tempfile.XXXXXX");

let result = mkstemp(&path);
match result {
Ok((fd, path)) => {
close(fd).unwrap();
unlink(path.as_path()).unwrap();
},
Err(e) => panic!("mkstemp failed: {}", e)
}
}

let result = mkstemp("/tmp/");
match result {
Ok(_) => {
panic!("mkstemp succeeded even though it should fail (provided a directory)");
},
Err(_) => {}
}
#[test]
fn test_mkstemp_directory() {
// mkstemp should fail if a directory is given
assert!(mkstemp(&env::temp_dir()).is_err());
}

#[test]
Expand Down