build: use status enum from task instead of success bool

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.
This commit is contained in:
Daiderd Jordan 2018-08-05 19:56:11 +02:00
parent a36543353e
commit 9d85f62835
No known key found for this signature in database
GPG key ID: D02435D05B810C96
4 changed files with 195 additions and 26 deletions

View file

@ -1,5 +1,15 @@
use ofborg::message::{Pr, Repo};
#[derive(Serialize, Deserialize, Debug)]
pub enum BuildStatus {
Skipped,
Success,
Failure,
TimedOut,
UnexpectedError { err: String },
}
#[derive(Serialize, Deserialize, Debug)]
pub struct BuildResult {
pub repo: Repo,
@ -8,7 +18,8 @@ pub struct BuildResult {
pub output: Vec<String>,
pub attempt_id: String,
pub request_id: Option<String>,
pub success: Option<bool>,
pub success: Option<bool>, // replaced by status
pub status: Option<BuildStatus>,
pub skipped_attrs: Option<Vec<String>>,
pub attempted_attrs: Option<Vec<String>>,
}

View file

@ -7,7 +7,7 @@ use uuid::Uuid;
use std::collections::VecDeque;
use ofborg::checkout;
use ofborg::message::buildjob;
use ofborg::message::buildresult;
use ofborg::message::buildresult::{BuildResult, BuildStatus};
use ofborg::message::buildlogmsg;
use ofborg::nix;
use ofborg::commentparser;
@ -112,7 +112,7 @@ impl<'a, 'b> JobActions<'a, 'b> {
}
pub fn merge_failed(&mut self) {
let msg = buildresult::BuildResult {
let msg = BuildResult {
repo: self.job.repo.clone(),
pr: self.job.pr.clone(),
system: self.system.clone(),
@ -121,6 +121,7 @@ impl<'a, 'b> JobActions<'a, 'b> {
request_id: Some(self.job.request_id.clone()),
attempted_attrs: None,
skipped_attrs: None,
status: Some(BuildStatus::Failure),
success: Some(false),
};
@ -195,7 +196,7 @@ impl<'a, 'b> JobActions<'a, 'b> {
pub fn build_not_attempted(&mut self, not_attempted_attrs: Vec<String>,
) {
let msg = buildresult::BuildResult {
let msg = BuildResult {
repo: self.job.repo.clone(),
pr: self.job.pr.clone(),
system: self.system.clone(),
@ -204,6 +205,7 @@ impl<'a, 'b> JobActions<'a, 'b> {
request_id: Some(self.job.request_id.clone()),
skipped_attrs: Some(not_attempted_attrs),
attempted_attrs: None,
status: None,
success: None,
};
@ -226,18 +228,23 @@ impl<'a, 'b> JobActions<'a, 'b> {
self.tell(worker::Action::Ack);
}
pub fn build_finished(&mut self, success: bool,
pub fn build_finished(&mut self, status: BuildStatus,
attempted_attrs: Vec<String>,
not_attempted_attrs: Vec<String>,
) {
let msg = buildresult::BuildResult {
let success = match status {
BuildStatus::Success => true,
_ => false,
};
let msg = BuildResult {
repo: self.job.repo.clone(),
pr: self.job.pr.clone(),
system: self.system.clone(),
output: self.log_snippet(),
attempt_id: self.attempt_id.clone(),
request_id: Some(self.job.request_id.clone()),
status: Some(status),
success: Some(success),
attempted_attrs: Some(attempted_attrs),
skipped_attrs: Some(not_attempted_attrs),
@ -373,21 +380,30 @@ impl notifyworker::SimpleNotifyWorker for BuildWorker {
actions.log_line(&line);
}
let success = match spawned.wait() {
Ok(status) => status.success(),
e => {
println!("Failed on the interior command: {:?}", e);
false
// TODO: this belongs in the nix module.
let status = match spawned.wait() {
Ok(s) => match s.code() {
Some(0) => BuildStatus::Success,
Some(100) => BuildStatus::Failure, // nix permanent failure
Some(101) => BuildStatus::TimedOut, // nix build timedout
Some(i) => BuildStatus::UnexpectedError {
err: format!("command failed with exit code {}", i),
},
None => BuildStatus::UnexpectedError {
err: "unexpected build failure".into(),
},
},
e => BuildStatus::UnexpectedError {
err: format!("failed on interior command {:?}", e),
}
};
println!("ok built ({:?}), building", success);
println!("ok built ({:?}), building", status);
println!("Lines:\n-----8<-----");
actions.log_snippet().iter().inspect(|x| println!("{}", x)).last();
println!("----->8-----");
actions.build_finished(success, can_build, cannot_build_attrs);
actions.build_finished(status, can_build, cannot_build_attrs);
println!("Done!");
}
}

View file

@ -4,7 +4,7 @@ extern crate env_logger;
use serde_json;
use hubcaps;
use ofborg::message::buildresult::BuildResult;
use ofborg::message::buildresult::{BuildStatus, BuildResult};
use ofborg::worker;
use amqp::protocol::basic::{Deliver, BasicProperties};
@ -86,13 +86,27 @@ fn result_to_comment(result: &BuildResult) -> String {
"".to_owned()
};
let status = match result.status {
None => {
// Fallback for old format.
match result.success {
None => &BuildStatus::Skipped,
Some(true) => &BuildStatus::Success,
Some(false) => &BuildStatus::Failure,
}
},
Some(ref s) => s,
};
reply.push(format!(
"{} on {}{}",
(match result.success {
Some(true) => "Success",
Some(false) => "Failure",
None => "No attempt",
}),
(match *status {
BuildStatus::Skipped => "No attempt".into(),
BuildStatus::Success => "Success".into(),
BuildStatus::Failure => "Failure".into(),
BuildStatus::TimedOut => "Timed out, unknown build status".into(),
BuildStatus::UnexpectedError { ref err } => format!("Unexpected error: {}", err),
}),
result.system,
log_link
));
@ -185,7 +199,8 @@ mod tests {
system: "x86_64-linux".to_owned(),
attempted_attrs: Some(vec!["foo".to_owned()]),
skipped_attrs: Some(vec!["bar".to_owned()]),
success: Some(true),
status: Some(BuildStatus::Success),
success: None,
};
assert_eq!(
@ -247,7 +262,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
system: "x86_64-linux".to_owned(),
attempted_attrs: Some(vec!["foo".to_owned()]),
skipped_attrs: None,
success: Some(false),
status: Some(BuildStatus::Failure),
success: None,
};
assert_eq!(
@ -276,6 +292,64 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
);
}
#[test]
pub fn test_timedout_build() {
let result = BuildResult {
repo: Repo {
clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(),
full_name: "NixOS/nixpkgs".to_owned(),
owner: "NixOS".to_owned(),
name: "nixpkgs".to_owned(),
},
pr: Pr {
head_sha: "abc123".to_owned(),
number: 2345,
target_branch: Some("master".to_owned()),
},
output: vec![
"make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
"make[2]: Nothing to be done for 'install'.".to_owned(),
"make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
"make[1]: Nothing to be done for 'install-target'.".to_owned(),
"make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(),
"removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(),
"post-installation fixup".to_owned(),
"building of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' timed out after 1 seconds".to_owned(),
"error: build of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' failed".to_owned(),
],
attempt_id: "neatattemptid".to_owned(),
request_id: Some("bogus-request-id".to_owned()),
system: "x86_64-linux".to_owned(),
attempted_attrs: Some(vec!["foo".to_owned()]),
skipped_attrs: None,
status: Some(BuildStatus::TimedOut),
success: None,
};
assert_eq!(
&result_to_comment(&result),
"Timed out, unknown build status on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid)
Attempted: foo
<details><summary>Partial log (click to expand)</summary><p>
```
make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
make[2]: Nothing to be done for 'install'.
make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
make[1]: Nothing to be done for 'install-target'.
make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'
removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'
post-installation fixup
building of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' timed out after 1 seconds
error: build of '/nix/store/l1limh50lx2cx45yb2gqpv7k8xl1mik2-gdb-8.1.drv' failed
```
</p></details>
"
);
}
#[test]
pub fn test_passing_build_unspecified_attributes() {
@ -308,7 +382,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
system: "x86_64-linux".to_owned(),
attempted_attrs: None,
skipped_attrs: None,
success: Some(true),
status: Some(BuildStatus::Success),
success: None,
};
assert_eq!(
@ -366,7 +441,8 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
system: "x86_64-linux".to_owned(),
attempted_attrs: None,
skipped_attrs: None,
success: Some(false),
status: Some(BuildStatus::Failure),
success: None,
};
assert_eq!(
@ -413,6 +489,7 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
system: "x86_64-linux".to_owned(),
attempted_attrs: None,
skipped_attrs: Some(vec!["not-attempted".to_owned()]),
status: Some(BuildStatus::Skipped),
success: None,
};
@ -453,6 +530,7 @@ foo
system: "x86_64-linux".to_owned(),
attempted_attrs: None,
skipped_attrs: Some(vec!["not-attempted".to_owned()]),
status: Some(BuildStatus::Skipped),
success: None,
};
@ -463,6 +541,69 @@ foo
The following builds were skipped because they don't evaluate on x86_64-linux: not-attempted
No partial log is available.
"
);
}
#[test]
pub fn test_no_status_compatibility() {
let result = BuildResult {
repo: Repo {
clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(),
full_name: "NixOS/nixpkgs".to_owned(),
owner: "NixOS".to_owned(),
name: "nixpkgs".to_owned(),
},
pr: Pr {
head_sha: "abc123".to_owned(),
number: 2345,
target_branch: Some("master".to_owned()),
},
output: vec![
"make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
"make[2]: Nothing to be done for 'install'.".to_owned(),
"make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
"make[1]: Nothing to be done for 'install-target'.".to_owned(),
"make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(),
"removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(),
"post-installation fixup".to_owned(),
"strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip".to_owned(),
"patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
"/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
],
attempt_id: "neatattemptid".to_owned(),
request_id: Some("bogus-request-id".to_owned()),
system: "x86_64-linux".to_owned(),
attempted_attrs: Some(vec!["foo".to_owned()]),
skipped_attrs: Some(vec!["bar".to_owned()]),
status: None,
success: Some(true),
};
assert_eq!(
&result_to_comment(&result),
"Success on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid)
Attempted: foo
The following builds were skipped because they don't evaluate on x86_64-linux: bar
<details><summary>Partial log (click to expand)</summary><p>
```
make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
make[2]: Nothing to be done for 'install'.
make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
make[1]: Nothing to be done for 'install-target'.
make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'
removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'
post-installation fixup
strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip
patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
```
</p></details>
"
);
}

View file

@ -9,7 +9,7 @@ use std::io::Write;
use ofborg::writetoline::LineWriter;
use ofborg::message::buildlogmsg::{BuildLogStart, BuildLogMsg};
use ofborg::message::buildresult::BuildResult;
use ofborg::message::buildresult::{BuildStatus, BuildResult};
use ofborg::worker;
use amqp::protocol::basic::{Deliver, BasicProperties};
@ -445,6 +445,7 @@ mod tests {
output: vec![],
attempt_id: "attempt-id-foo".to_owned(),
request_id: Some("bogus-request-id".to_owned()),
status: Some(BuildStatus::Success),
success: Some(true),
attempted_attrs: Some(vec!["foo".to_owned()]),
skipped_attrs: Some(vec!["bar".to_owned()]),
@ -478,6 +479,6 @@ mod tests {
let mut s = String::new();
pr.push("routing-key-foo/attempt-id-foo.result.json");
File::open(pr).unwrap().read_to_string(&mut s).unwrap();
assert_eq!(&s, "{\"repo\":{\"owner\":\"NixOS\",\"name\":\"ofborg\",\"full_name\":\"NixOS/ofborg\",\"clone_url\":\"https://github.com/nixos/ofborg.git\"},\"pr\":{\"target_branch\":\"scratch\",\"number\":42,\"head_sha\":\"6dd9f0265d52b946dd13daf996f30b64e4edb446\"},\"system\":\"x86_64-linux\",\"output\":[],\"attempt_id\":\"attempt-id-foo\",\"request_id\":\"bogus-request-id\",\"success\":true,\"skipped_attrs\":[\"bar\"],\"attempted_attrs\":[\"foo\"]}");
assert_eq!(&s, "{\"repo\":{\"owner\":\"NixOS\",\"name\":\"ofborg\",\"full_name\":\"NixOS/ofborg\",\"clone_url\":\"https://github.com/nixos/ofborg.git\"},\"pr\":{\"target_branch\":\"scratch\",\"number\":42,\"head_sha\":\"6dd9f0265d52b946dd13daf996f30b64e4edb446\"},\"system\":\"x86_64-linux\",\"output\":[],\"attempt_id\":\"attempt-id-foo\",\"request_id\":\"bogus-request-id\",\"success\":true,\"status\":\"Success\",\"skipped_attrs\":[\"bar\"],\"attempted_attrs\":[\"foo\"]}");
}
}