Bug 1443853 - Rename RunnerProcess::status() to ::try_wait(). r?jgraham
This renames RunnerProcess::status() to ::try_wait() for symmetry
with std::process::Child::try_wait() in the standard library.
The patch also makes an attempt at cleaning up its usage in geckodriver,
however this can be further improved.
MozReview-Commit-ID: 14ihT7MpM7l
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -1336,64 +1336,69 @@ pub struct MarionetteConnection {
pub session: MarionetteSession
}
impl MarionetteConnection {
pub fn new(port: u16, session_id: Option<String>) -> MarionetteConnection {
MarionetteConnection {
port: port,
stream: None,
- session: MarionetteSession::new(session_id)
+ session: MarionetteSession::new(session_id),
}
}
pub fn connect(&mut self, browser: &mut Option<FirefoxProcess>) -> WebDriverResult<()> {
- let timeout = 60 * 1000; // ms
- let poll_interval = 100; // ms
+ let timeout = 60 * 1000; // ms
+ let poll_interval = 100; // ms
let poll_attempts = timeout / poll_interval;
let mut poll_attempt = 0;
loop {
- // If the process is gone, immediately abort the connection attempts
+ // immediately abort connection attempts if process disappears
if let &mut Some(ref mut runner) = browser {
- let status = runner.status();
- if status.is_err() || status.as_ref().map(|x| *x).unwrap_or(None) != None {
+ let exit_status = match runner.try_wait() {
+ Ok(Some(status)) => Some(
+ status
+ .code()
+ .map(|c| c.to_string())
+ .unwrap_or("signal".into()),
+ ),
+ Ok(None) => None,
+ Err(_) => Some("{unknown}".into()),
+ };
+ if let Some(s) = exit_status {
return Err(WebDriverError::new(
ErrorStatus::UnknownError,
- format!("Process unexpectedly closed with status: {}", status
- .ok()
- .and_then(|x| x)
- .and_then(|x| x.code())
- .map(|x| x.to_string())
- .unwrap_or("{unknown}".into()))));
+ format!("Process unexpectedly closed with status {}", s),
+ ));
}
}
match TcpStream::connect(&(DEFAULT_HOST, self.port)) {
Ok(stream) => {
self.stream = Some(stream);
- break
- },
+ break;
+ }
Err(e) => {
trace!(" connection attempt {}/{}", poll_attempt, poll_attempts);
if poll_attempt <= poll_attempts {
poll_attempt += 1;
sleep(Duration::from_millis(poll_interval));
} else {
return Err(WebDriverError::new(
- ErrorStatus::UnknownError, e.description().to_owned()));
+ ErrorStatus::UnknownError,
+ e.description().to_owned(),
+ ));
}
}
}
- };
+ }
debug!("Connected to Marionette on {}:{}", DEFAULT_HOST, self.port);
-
- try!(self.handshake());
- Ok(())
+ self.handshake()
}
fn handshake(&mut self) -> WebDriverResult<()> {
let resp = try!(self.read_resp());
let handshake_data = try!(Json::from_str(&*resp));
let data = try_opt!(handshake_data.as_object(),
ErrorStatus::UnknownError,
--- a/testing/mozbase/rust/mozrunner/src/runner.rs
+++ b/testing/mozbase/rust/mozrunner/src/runner.rs
@@ -42,17 +42,27 @@ pub trait Runner {
fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self
where
T: Into<Stdio>;
fn start(self) -> Result<Self::Process, RunnerError>;
}
pub trait RunnerProcess {
- fn status(&mut self) -> io::Result<Option<process::ExitStatus>>;
+ /// Attempts to collect the exit status of the process if it has already exited.
+ ///
+ /// This function will not block the calling thread and will only advisorily check to see if
+ /// the child process has exited or not. If the process has exited then on Unix the process ID
+ /// is reaped. This function is guaranteed to repeatedly return a successful exit status so
+ /// long as the child has already exited.
+ ///
+ /// If the process has exited, then `Ok(Some(status))` is returned. If the exit status is not
+ /// available at this time then `Ok(None)` is returned. If an error occurs, then that error is
+ /// returned.
+ fn try_wait(&mut self) -> io::Result<Option<process::ExitStatus>>;
/// Determine if the process is still running.
fn running(&mut self) -> bool;
/// Forces the process to exit and returns the exit status. This is
/// equivalent to sending a SIGKILL on Unix platforms.
fn kill(&mut self) -> io::Result<process::ExitStatus>;
}
@@ -104,22 +114,22 @@ impl From<PrefReaderError> for RunnerErr
#[derive(Debug)]
pub struct FirefoxProcess {
process: Child,
profile: Profile,
}
impl RunnerProcess for FirefoxProcess {
- fn status(&mut self) -> io::Result<Option<process::ExitStatus>> {
+ fn try_wait(&mut self) -> io::Result<Option<process::ExitStatus>> {
self.process.try_wait()
}
fn running(&mut self) -> bool {
- self.status().unwrap().is_none()
+ self.try_wait().unwrap().is_none()
}
fn kill(&mut self) -> io::Result<process::ExitStatus> {
self.process.kill()?;
self.process.wait()
}
}