[snowpatch] [PATCH v3 3/3] settings: improve parsing of jobs

Andrew Donnellan andrew.donnellan at au1.ibm.com
Fri Mar 24 14:28:27 AEDT 2017


Job entries in the config file are a mixture of parameters which should be
passed straight through to Jenkins, and flags that have meaning to
snowpatch.

I don't really feel like splitting these two things apart in the config, as
we already have enough levels of nesting. Instead, add our own deserialize
method to do this more intelligently. This also serves as an example of how
to do more advanced parsing such that we can reject invalid values at parse
time.

Thanks-to: David Tolnay <dtolnay at gmail.com> # help with boilerplate code
Signed-off-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
---
v2->v3:
* I figure I should just throw this patch in now...

---
 src/main.rs     | 28 ++++++++---------
 src/settings.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 98 insertions(+), 25 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 6ba550e..a0e6576 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -108,22 +108,18 @@ fn run_tests(settings: &Config, client: Arc<Client>, project: &Project, tag: &st
         token: settings.jenkins.token.clone(),
     };
     let project = project.clone();
-    for job_params in &project.jobs {
-        let job_name = job_params.get("job").unwrap();
-        let job_title = settings::get_job_title(job_params);
+    for job in &project.jobs {
         let mut jenkins_params = Vec::<(&str, &str)>::new();
-        for (param_name, param_value) in job_params.iter() {
+        for (param_name, param_value) in job.parameters.iter() {
+            // TODO(ajd): do this more neatly
             debug!("Param name {}, value {}", &param_name, &param_value);
-            match param_name.as_ref() {
-                // TODO: Validate special parameter names in config at start of program
-                "job" | "title" => { },
-                "remote" => jenkins_params.push((param_value, &project.remote_uri)),
-                "branch" => jenkins_params.push((param_value, tag)),
-                _ => jenkins_params.push((param_name, param_value)),
-            }
+            jenkins_params.push((param_name, param_value));
         }
-        info!("Starting job: {}", &job_title);
-        let res = jenkins.start_test(job_name, jenkins_params)
+        jenkins_params.push((&job.remote, &project.remote_uri));
+        jenkins_params.push((&job.branch, tag));
+
+        info!("Starting job: {}", &job.title);
+        let res = jenkins.start_test(&job.job, jenkins_params)
             .unwrap_or_else(|err| panic!("Starting Jenkins test failed: {}", err));
         debug!("{:?}", &res);
         let build_url_real;
@@ -137,12 +133,12 @@ fn run_tests(settings: &Config, client: Arc<Client>, project: &Project, tag: &st
         debug!("Build URL: {}", build_url_real);
         jenkins.wait_build(&build_url_real);
         let test_result = jenkins.get_build_result(&build_url_real).unwrap();
-        info!("Jenkins job for {}/{} complete.", branch_name, job_title);
+        info!("Jenkins job for {}/{} complete.", branch_name, job.title);
         results.push(TestResult {
-            test_name: format!("Test {} on branch {}", job_title.to_string(),
+            test_name: format!("Test {} on branch {}", job.title,
                                branch_name.to_string()).to_string(),
             state: test_result,
-            url: Some(jenkins.get_results_url(&build_url_real, job_params)),
+            url: Some(jenkins.get_results_url(&build_url_real, &job.parameters)),
             summary: Some("TODO: get this summary from Jenkins".to_string()),
         });
     }
diff --git a/src/settings.rs b/src/settings.rs
index 826f792..44c321c 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -16,8 +16,11 @@
 
 use toml;
 
+use serde::de::{self, MapVisitor, Visitor, Deserializer, Deserialize};
+
 use git2::{Repository, Error};
 
+use std::fmt;
 use std::fs::File;
 use std::io::Read;
 use std::collections::BTreeMap;
@@ -58,8 +61,8 @@ pub struct Project {
     pub test_all_branches: Option<bool>,
     pub remote_name: String,
     pub remote_uri: String,
-    pub jobs: Vec<BTreeMap<String, String>>,
-    pub push_results: bool
+    pub jobs: Vec<Job>,
+    pub push_results: bool,
 }
 
 impl Project {
@@ -68,6 +71,87 @@ impl Project {
     }
 }
 
+#[derive(Clone)]
+pub struct Job {
+    pub job: String,
+    pub title: String,
+    pub remote: String,
+    pub branch: String,
+    pub parameters: BTreeMap<String, String>,
+}
+
+impl Deserialize for Job {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+        where D: Deserializer
+    {
+        struct JobVisitor;
+
+        impl Visitor for JobVisitor {
+            type Value = Job;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("struct Job with arbitrary fields")
+            }
+
+            fn visit_map<V>(self, mut visitor: V) -> Result<Job, V::Error>
+                where V: MapVisitor
+            {
+                let mut job = None;
+                let mut title = None;
+                let mut remote = None;
+                let mut branch = None;
+                let mut parameters = BTreeMap::new();
+                while let Some(key) = visitor.visit_key::<String>()? {
+                    match key.as_str() {
+                        "job" => {
+                            if job.is_some() {
+                                return Err(de::Error::duplicate_field("job"));
+                            }
+                            job = Some(visitor.visit_value()?);
+                        }
+                        "title" => {
+                            if title.is_some() {
+                                return Err(de::Error::duplicate_field("title"));
+                            }
+                            title = Some(visitor.visit_value()?);
+                        }
+                        "remote" => {
+                            if remote.is_some() {
+                                return Err(de::Error::duplicate_field("remote"));
+                            }
+                            remote = Some(visitor.visit_value()?);
+                        }
+                        "branch" => {
+                            if branch.is_some() {
+                                return Err(de::Error::duplicate_field("branch"));
+                            }
+                            branch = Some(visitor.visit_value()?);
+                        }
+                        _ => {
+                            parameters.insert(key, visitor.visit_value()?);
+                        }
+                    }
+                }
+
+                let job: String = job.ok_or_else(|| de::Error::missing_field("job"))?;
+                let remote = remote.ok_or_else(|| de::Error::missing_field("remote"))?;
+                let branch = branch.ok_or_else(|| de::Error::missing_field("branch"))?;
+                let title = title.unwrap_or(job.clone());
+
+                Ok(Job {
+                    job: job,
+                    title: title,
+                    remote: remote,
+                    branch: branch,
+                    parameters: parameters,
+                })
+            }
+        }
+
+        deserializer.deserialize_map(JobVisitor)
+    }
+}
+
 #[derive(Deserialize, Clone)]
 pub struct Config {
     pub git: Git,
@@ -76,13 +160,6 @@ pub struct Config {
     pub projects: BTreeMap<String, Project>
 }
 
-pub fn get_job_title(job: &BTreeMap<String, String>) -> String {
-    match job.get("title") {
-        Some(title) => title.to_string(),
-        None => job.get("job").unwrap().to_string()
-    }
-}
-
 pub fn parse(path: &str) -> Config {
     let mut toml_config = String::new();
 
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited



More information about the snowpatch mailing list