[summary] Make PR comment pretty

Cargo.toml

@@ -93,6 +93,7 @@ getrandom = "0.2.12"
 git2 = "0.19.0"
 hex = "0.4.3"
 http = "1.1.0"
+humantime = "2.1.0"
 glob = "0.3.1"
 gloo-storage = "0.3.0"
 gloo-utils = "0.2.0"

linera-summary/Cargo.toml

@@ -15,11 +15,11 @@ edition.workspace = true
 anyhow.workspace = true
 clap.workspace = true
 git2.workspace = true
+humantime.workspace = true
 linera-base.workspace = true
 linera-version.workspace = true
 octocrab.workspace = true
 serde.workspace = true
-serde_json.workspace = true
 tokio.workspace = true
 tracing.workspace = true
 

linera-summary/src/ci_runtime_comparison.rs

@@ -1,7 +1,7 @@
 // Copyright (c) Zefchain Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 use anyhow::{ensure, Result};
 use octocrab::models::workflows::{Conclusion, Job, Status};
@@ -11,21 +11,43 @@ use tracing::warn;
 type WorkflowName = String;
 
 #[derive(Serialize)]
-struct WorkflowJobRuntimeComparison {
+pub struct WorkflowJobRuntimeComparison {
     name: String,
     workflow_name: String,
-    base_runtime: String,
-    pr_runtime: String,
-    runtime_difference_pct: String,
+    base_runtime: u64,
+    pr_runtime: u64,
+    runtime_difference_pct: f64,
+}
+
+impl WorkflowJobRuntimeComparison {
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
+    pub fn workflow_name(&self) -> &str {
+        &self.workflow_name
+    }
+
+    pub fn base_runtime(&self) -> u64 {
+        self.base_runtime
+    }
+
+    pub fn pr_runtime(&self) -> u64 {
+        self.pr_runtime
+    }
+
+    pub fn runtime_difference_pct(&self) -> f64 {
+        self.runtime_difference_pct
+    }
 }
 
 // The key is the name of the workflow, and the value is a list of per job comparisons.
 #[derive(Serialize)]
-pub struct CiRuntimeComparison(HashMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>);
+pub struct CiRuntimeComparison(pub BTreeMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>);
 
 impl CiRuntimeComparison {
-    fn get_runtimes(jobs: Vec<Job>) -> Result<HashMap<String, HashMap<String, u64>>> {
-        let mut runtimes: HashMap<String, HashMap<String, u64>> = HashMap::new();
+    fn get_runtimes(jobs: Vec<Job>) -> Result<BTreeMap<String, BTreeMap<String, u64>>> {
+        let mut runtimes: BTreeMap<String, BTreeMap<String, u64>> = BTreeMap::new();
         for job in jobs {
             ensure!(job.status == Status::Completed);
             ensure!(job.conclusion.is_some());
@@ -62,14 +84,12 @@ impl CiRuntimeComparison {
                                         return Some(WorkflowJobRuntimeComparison {
                                             name: job_name.clone(),
                                             workflow_name: workflow_name.clone(),
-                                            pr_runtime: format!("{}s", pr_runtime),
-                                            base_runtime: format!("{}s", *base_runtime),
-                                            runtime_difference_pct: format!(
-                                                "{:+.2}%",
+                                            pr_runtime,
+                                            base_runtime: *base_runtime,
+                                            runtime_difference_pct:
                                                 ((pr_runtime as f64) / (*base_runtime as f64)
                                                     - 1.0)
-                                                    * 100.0
-                                            ),
+                                                    * 100.0,
                                         });
                                     } else {
                                         warn!(
@@ -89,7 +109,7 @@ impl CiRuntimeComparison {
                             .collect::<Vec<_>>(),
                     )
                 })
-                .collect::<HashMap<_, _>>(),
+                .collect::<BTreeMap<_, _>>(),
         ))
     }
 }

linera-summary/src/github.rs

@@ -15,12 +15,20 @@ use tracing::info;
 const API_REQUEST_DELAY_MS: u64 = 100;
 const IGNORED_JOB_PREFIXES: &[&str] = &["lint-", "check-outdated-cli-md"];
 
-struct GithubRepository {
+pub struct GithubRepository {
     owner: String,
     name: String,
 }
 
 impl GithubRepository {
+    pub fn owner(&self) -> &str {
+        &self.owner
+    }
+
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
     fn from_env(is_local: bool) -> Result<Self> {
         let env_repository = env::var("GITHUB_REPOSITORY");
         let repository = if is_local {
@@ -59,6 +67,10 @@ impl GithubContext {
         &self.pr_commit_hash
     }
 
+    pub fn repository(&self) -> &GithubRepository {
+        &self.repository
+    }
+
     fn get_local_git_info() -> Result<(String, String, String)> {
         let repo = git2::Repository::open_from_env().context("Failed to open git repository")?;
 
@@ -152,7 +164,7 @@ impl Github {
         &self.context
     }
 
-    pub async fn comment_on_pr(&self, body: &str) -> Result<()> {
+    pub async fn comment_on_pr(&self, body: String) -> Result<()> {
         if self.is_local {
             info!("Printing summary to stdout:");
             println!("{}", body);

linera-summary/src/performance_summary.rs

@@ -1,9 +1,10 @@
 // Copyright (c) Zefchain Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::collections::HashSet;
+use std::{collections::HashSet, time::Duration};
 
-use anyhow::Result;
+use anyhow::{bail, Result};
+use humantime::format_duration;
 use serde::Serialize;
 
 use crate::{ci_runtime_comparison::CiRuntimeComparison, github::Github};
@@ -24,35 +25,82 @@ impl PerformanceSummary {
             .into_iter()
             .filter(|workflow| tracked_workflows.contains(&workflow.name))
             .collect::<Vec<_>>();
+
         let base_jobs = Box::pin(github.latest_jobs(
             github.context().base_branch(),
             "push",
             &workflows_handler,
             &workflows,
         ))
         .await?;
+        if base_jobs.is_empty() {
+            bail!("No base jobs found!");
+        }
+
         let pr_jobs = Box::pin(github.latest_jobs(
             github.context().pr_branch(),
             "pull_request",
             &workflows_handler,
             &workflows,
         ))
         .await?;
+        if pr_jobs.is_empty() {
+            bail!("No PR jobs found!");
+        }
 
         Ok(Self {
             github,
             ci_runtime_comparison: CiRuntimeComparison::from_jobs(base_jobs, pr_jobs)?,
         })
     }
 
-    pub async fn comment_on_pr(&self) -> Result<()> {
-        let body = format!(
-            "## CI Performance Summary for commit {}\n\n```json\n{}\n```",
-            &self.github.context().pr_commit_hash()[..7],
-            serde_json::to_string_pretty(self)?
+    fn format_comment_body(&self) -> String {
+        let commit_hash = self.github.context().pr_commit_hash();
+        let short_commit_hash = &commit_hash[..7];
+        let commit_url = format!(
+            "https://github.com/{}/{}/commit/{}",
+            self.github.context().repository().owner(),
+            self.github.context().repository().name(),
+            commit_hash
         );
 
-        self.github.jparrowsec.cnment_on_pr(&body).await?;
+        let mut markdown_content = format!(
+            "## Performance Summary for commit [{}]({})\n\n",
+            short_commit_hash, commit_url
+        );
+
+        markdown_content.push_str("### CI Runtime Comparison\n\n");
+        for (workflow_name, comparisons) in self.ci_runtime_comparison.0.iter() {
+            markdown_content.push_str(&format!("#### Workflow: {}\n\n", workflow_name));
+            markdown_content
+                .push_str("| Job Name | Base Runtime | PR Runtime | Runtime Difference (%) |\n");
+            markdown_content.push_str("| --- | --- | --- | --- |\n");
+
+            for comparison in comparisons {
+                let base_runtime =
+                    format_duration(Duration::from_secs(comparison.base_runtime())).to_string();
+                let pr_runtime =
+                    format_duration(Duration::from_secs(comparison.pr_runtime())).to_string();
+                let runtime_difference_pct = format!("{:.2}%", comparison.runtime_difference_pct());
+
+                markdown_content.push_str(&format!(
+                    "| {} | {} | {} | {} |\n",
+                    comparison.name(),
+                    base_runtime,
+                    pr_runtime,
+                    runtime_difference_pct
+                ));
+            }
+
+            markdown_content.push('\n');
+        }
+        markdown_content
+    }
+
+    pub async fn comment_on_pr(&self) -> Result<()> {
+        self.github
+            .comment_on_pr(self.format_comment_body())
+            .await?;
         Ok(())
     }
 }