diff options
-rw-r--r-- | mod/mod-ci-github-service-data.hxx | 12 | ||||
-rw-r--r-- | mod/mod-ci-github.cxx | 35 |
2 files changed, 29 insertions, 18 deletions
diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx index 21c8a8f..5573d90 100644 --- a/mod/mod-ci-github-service-data.hxx +++ b/mod/mod-ci-github-service-data.hxx @@ -24,12 +24,14 @@ namespace brep // struct check_run { - string build_id; // Full build id. - string name; // Potentially shortened build id. - optional<string> node_id; // GitHub id. + string build_id; // Full build id. + string name; // Potentially shortened build id. + optional<string> node_id; // GitHub id. - build_state state; - bool state_synced; + build_state state; + bool state_synced; + + optional<result_status> status; // Only present if state is built. string state_string () const diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 70c1c7d..8f1aa76 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -32,8 +32,6 @@ // Will need to extract a few more fields from check_runs, but the layout // is very similar to that of check_suite. // -// - Pull requests. Handle -// // - Choose strong webhook secret (when deploying). // // - Check that delivery UUID has not been received before (replay attack). @@ -283,8 +281,6 @@ namespace brep // @@ There is also check_run even (re-requested by user, either // individual check run or all the failed check runs). // - // @@ There is also the pull_request event. Probably need to handle. - // if (event == "check_suite") { gh_check_suite_event cs; @@ -310,10 +306,10 @@ namespace brep } else if (cs.action == "rerequested") { - // Someone manually requested to re-run the check runs in this check - // suite. Treat as a new request. + // Someone manually requested to re-run all the check runs in this + // check suite. Treat as a new request. // - // @@ This is probably broken. + // @@@ Test and make sure works? // return handle_check_suite_request (move (cs), warning_success); } @@ -517,6 +513,7 @@ namespace brep // want to run this code as early as possible to minimize the window of // the user seeing misleading CI results. // + if (cs.action == "requested") { // Fetch open pull requests with the check suite's head branch as base // branch. @@ -679,6 +676,10 @@ namespace brep // end up in an important branch (as opposed to the head of a feature // branch). // + // Note that in these two cases we are building different commit (the + // head commit vs merge commit). So it's not clear how we can have + // a single check_suite result represent both? + // // Possible solution: ignore all check_suites with non-empty // pull_requests[]. // @@ -1551,8 +1552,11 @@ namespace brep return nullptr; } + // Absent if have any unbuilt check runs. + // + optional<result_status> conclusion (*b.status); + check_run cr; // Updated check run. - bool unbuilt (false); // True if we have any other unbuilt check runs. { string bid (gh_check_run_name (b)); // Full Build ID. @@ -1566,11 +1570,16 @@ namespace brep } else { - if (cr.state != build_state::built) - unbuilt = true; + if (cr.state == build_state::built) + { + if (conclusion) + *conclusion |= cr.status + } + else + conclusion = nullopt; } - if (scr && unbuilt) + if (scr && !conclusion.has_value ()) break; } @@ -1775,13 +1784,13 @@ namespace brep // Update the conclusion check run if all check runs are now built. // - if (cr.state == build_state::built && !unbuilt) + if (cr.state == build_state::built && conclusion) { assert (sd.conclusion_node_id); // Update the conclusion check run with success. // - result_status rs (result_status::success); + result_status rs (*conclusion); optional<gq_built_result> br ( gq_built_result (gh_to_conclusion (rs, sd.warning_success), |