-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Ready to enable android bot to make check 2 #10458
Conversation
fn test_destroy_once() { | ||
let mut p = run::Process::new("echo", [], run::ProcessOptions::new()); | ||
p.destroy(); // this shouldn't crash (and nor should the destructor) | ||
} | ||
#[cfg(target_os="android")] | ||
fn test_destroy_once() { | ||
let mut p = run::Process::new("/system/bin/sh -c echo", [], run::ProcessOptions::new()); |
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.
How does this run? The first argument is the program to run, which in this case the file /system/bin/sh -c echo
shouldn't really exist.
Also, would it be possible to not duplicate all of the tests? I'd rather have a fn args(&str, ~[~str]) -> (&str, ~[~str])
that's the identity function on unix but this /system/bin/sh
stuff on android.
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.
It runs. Since echo is not provided as binary but shell functionality. "/system/bin/sh -c echo" is passed as shell command on android.
I will find a way not to duplicate but this patch is focus on just pass android make check.
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.
Shouldn't it at least be run::Process::new("/system/bin/sh", [~"-c", ~"echo"], ...)
?
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.
Oh I certainly agree that the patch is fine, but this duplication of process tests on android vs non-android unix is prevalent in many locations, and if we're serious about running the tests on all platforms then we should start moving towards a cleaner solution of generating tests I was thinking.
To enable test on android bot #9120 workcache::test disabled and run-pass/core-run-destroy.rs fixed on android
@brson how is it going about arm bot ? |
…k, r=llogiq lintcheck: use multithreading unless --fix or --recursive is used Use multithreading unless there is a reason not to. changelog: none
To enable test on android bot #9120
workcache::test disabled and run-pass/core-run-destroy.rs fixed on android