-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
detect timedout builds #206
Conversation
This allows the commentposter to handle other build statuses like a timeout. The BuildResult struct still includes success and the commentposter can still handle messages in the old format for backwards compatibility.
assert_eq!(lines, vec!["stdout", "stderr", "stdout2", "stderr2"]); | ||
let ret = spawned.wait().unwrap().unwrap().unwrap().success(); |
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.
😍
}, | ||
}, | ||
e => BuildStatus::UnexpectedError { | ||
err: format!("failed on interior command {:?}", e), |
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 don't think it's a problem in practice, but this could potentially expose more information about the builder then before.
self.waiter.join() | ||
.map_err(|_err| io::Error::new(io::ErrorKind::Other, "Couldn't join thread.")) | ||
.and_then(|opt| opt.ok_or(io::Error::new(io::ErrorKind::Other, "Thread didn't return an exit status."))) |
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'm not entirely sure these messages are correct.
This should change the error messages for timed out builds to something like.
I think this is fully backwards compatible, allowing builders to continue sending build results in the old format, while newer versions send more information using the new format. This could be cleaned up once all the infrastructure is updated.