Merge pull request #199 from Emilgardis/improve-ci

Improve ci

Author: Emilgardis

.gitlab-ci.yml

@@ -12,6 +12,7 @@ regress:nightly:
   image: rustlang/rust:nightly
   script:
     - cargo build --release
+    - rustup component add rustfmt-preview
     - cd ci/svd2rust-regress
     - rm -rf ./output
-    - cargo run --release -- --long-test
+    - cargo run --release -- --long-test --format --verbose

ci/svd2rust-regress/Cargo.toml

@@ -1,11 +1,11 @@
 [package]
 name = "svd2rust-regress"
 version = "0.1.0"
-authors = ["James Munns <[email protected]>"]
+authors = ["James Munns <[email protected]>", "The svd2rust developers"]
 
 [dependencies]
 reqwest = "0.8"
 rayon = "1.0"
 structopt = "0.2"
 error-chain = "0.11"
-inflections = "1.1.0"
+inflections = "1.1.0"

ci/svd2rust-regress/README.md

@@ -15,7 +15,10 @@
 
 ### Preconditions
 
-By default, `svd2rust-regress` assumes you have already built `svd2rust` in the root of this repository in `--release` mode. If this is not possible, it is possible to specify the path to an `svd2rust` binary (see **Options** below).
+By default, `svd2rust-regress` assumes you have already built `svd2rust` in the root of this repository in `--release` mode.
+If this is not possible, it is possible to specify the path to an `svd2rust` binary (see **Options** below).
+
+You'll also need to have `rustfmt` version > v0.4.0 to use the `--format` flag, you can install `rustfmt` with `rustup component add rustfmt-preview`.
 
 ### Output
 
@@ -30,7 +33,7 @@ Passed: spansion_mb9af12x_k - 23 seconds
 Fail results look like this:
 
 ```text
-Failed: si_five_e310x - 0 seconds - Error(Msg("Process Failed! - cargo check"), State { next_error: None, backtrace: None })
+Failed: si_five_e310x - 0 seconds. Error: Process Failed - cargo check
 ```
 
 If all test cases passed, the return code will be `0`. If any test cases failed, the return code will be `1`.
@@ -42,24 +45,32 @@ Here are the options for running `svd2rust-regress`:
 
 ```text
 svd2rust-regress 0.1.0
-James Munns <[email protected]>
+James Munns <[email protected]>:The svd2rust developers
 
 USAGE:
     svd2rust-regress [FLAGS] [OPTIONS]
 
 FLAGS:
     -b, --bad-tests    Include tests expected to fail (will cause a non-zero return code)
+    -f, --format       Enable formatting with `rustfmt`
     -h, --help         Prints help information
     -l, --long-test    Run a long test (it's very long)
     -V, --version      Prints version information
+    -v, --verbose      Use verbose output
 
 OPTIONS:
-    -a, --architecture <arch>         Filter by architecture, case sensitive, may be combined with other filters Options
-                                      are: "CortexM", "RiscV", and "Msp430"
-    -p, --svd2rust-path <bin_path>    Path to an `svd2rust` binary, relative or absolute. Defaults to
-                                      `target/release/svd2rust[.exe]` of this repository (which must be already built)
-    -c, --chip <chip>                 Filter by chip name, case sensitive, may be combined with other filters
-    -m, --manufacturer <mfgr>         Filter by manufacturer, case sensitive, may be combined with other filters
+    -a, --architecture <arch>
+            Filter by architecture, case sensitive, may be combined with other filters Options are: "CortexM", "RiscV",
+            and "Msp430"
+    -p, --svd2rust-path <bin_path>
+            Path to an `svd2rust` binary, relative or absolute. Defaults to `target/release/svd2rust[.exe]` of this
+            repository (which must be already built)
+    -c, --chip <chip>                            Filter by chip name, case sensitive, may be combined with other filters
+    -m, --manufacturer <mfgr>
+            Filter by manufacturer, case sensitive, may be combined with other filters
+
+        --rustfmt_bin_path <rustfmt_bin_path>
+            Path to an `rustfmt` binary, relative or absolute. Defaults to `$(rustup which rustfmt)`
 ```
 
 ### Filters
@@ -92,4 +103,6 @@ cargo run --release -- --long-test -c MB9AF12xK -m Fujitsu
     Finished release [optimized] target(s) in 0.0 secs
      Running `target/release/svd2rust-regress --long-test -c MB9AF12xK -m Fujitsu`
 Passed: fujitsu_mb9af12x_k - 19 seconds
-```
+```
+
+Note that you may have to pass `--long-test` to enable some chips as they are known to take a long time to compile.

ci/svd2rust-regress/src/errors.rs

@@ -1 +1,9 @@
-error_chain!{}
+use std::path::PathBuf;
+error_chain!{
+    errors {
+        ProcessFailed(command: String, stderr: Option<PathBuf>, stdout: Option<PathBuf>) {
+            description("Process Failed")
+            display("Process Failed - {}", command)
+        }
+    }
+}

ci/svd2rust-regress/src/main.rs

@@ -6,16 +6,19 @@ extern crate reqwest;
 #[macro_use]
 extern crate structopt;
 
-mod tests;
 mod errors;
 mod svd_test;
+mod tests;
 
-use std::path::PathBuf;
-use structopt::StructOpt;
+use error_chain::ChainedError;
 use rayon::prelude::*;
+use std::fs::File;
+use std::io::Read;
+use std::path::PathBuf;
+use std::process::{exit, Command};
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::process::exit;
 use std::time::Instant;
+use structopt::StructOpt;
 
 #[derive(StructOpt, Debug)]
 #[structopt(name = "svd2rust-regress")]
@@ -47,6 +50,18 @@ struct Opt {
     #[structopt(short = "b", long = "bad-tests")]
     bad_tests: bool,
 
+    /// Enable formatting with `rustfmt`
+    #[structopt(short = "f", long = "format")]
+    format: bool,
+
+    /// Path to an `rustfmt` binary, relative or absolute.
+    /// Defaults to `$(rustup which rustfmt)`
+    #[structopt(long = "rustfmt_bin_path", parse(from_os_str))]
+    rustfmt_bin_path: Option<PathBuf>,
+
+    /// Use verbose output
+    #[structopt(long = "verbose", short = "v", parse(from_occurrences))]
+    verbose: u8,
     // TODO: Specify smaller subset of tests? Maybe with tags?
     // TODO: Early fail
     // TODO: Compile svd2rust?
@@ -93,6 +108,36 @@ fn main() {
         None => &default_svd2rust,
     };
 
+    let default_rustfmt: Option<PathBuf> = if let Some((v, true)) = Command::new("rustup")
+        .args(&["which", "rustfmt"])
+        .output()
+        .ok()
+        .map(|o| (o.stdout, o.status.success()))
+    {
+        Some(String::from_utf8_lossy(&v).into_owned().trim().into())
+    } else {
+        if opt.format && opt.rustfmt_bin_path.is_none() {
+            panic!("rustfmt binary not found, is rustup and rustfmt-preview installed?");
+        }
+        None
+    };
+
+    let rustfmt_bin_path = match (&opt.rustfmt_bin_path, opt.format) {
+        (_, false) => None,
+        (&Some(ref path), true) => Some(path),
+        (&None, true) => {
+            // FIXME: Use Option::filter instead when stable, rust-lang/rust#45860
+            if default_rustfmt
+                .iter()
+                .filter(|p| p.is_file())
+                .next()
+                .is_none()
+            {
+                panic!("No rustfmt found");
+            }
+            default_rustfmt.as_ref()
+        }
+    };
     // collect enabled tests
     let tests = tests::TESTS
         .iter()
@@ -133,8 +178,9 @@ fn main() {
     tests.par_iter().for_each(|t| {
         let start = Instant::now();
 
-        match svd_test::test(t, &bin_path) {
+        match svd_test::test(t, &bin_path, rustfmt_bin_path) {
             Ok(()) => {
+                // TODO: If verbosity is > 1, print every logged stderr
                 eprintln!(
                     "Passed: {} - {} seconds",
                     t.name(),
@@ -143,11 +189,31 @@ fn main() {
             }
             Err(e) => {
                 any_fails.store(true, Ordering::Release);
+                let additional_info = if opt.verbose > 0 {
+                    match e.kind() {
+                        &errors::ErrorKind::ProcessFailed(ref command, _, Some(ref stderr))
+                            if command == "cargo check" =>
+                        {
+                            let mut buf = String::new();
+                            // Unwrap is safe
+                            File::open(stderr)
+                                .expect("Couldn't open file")
+                                .read_to_string(&mut buf)
+                                .expect("Couldn't read file to string");
+                            buf.insert_str(0, &format!("\n---{:?}---\n", stderr.as_os_str()));
+                            buf
+                        }
+                        _ => "".into(),
+                    }
+                } else {
+                    "".into()
+                };
                 eprintln!(
-                    "Failed: {} - {} seconds - {:?}",
+                    "Failed: {} - {} seconds. {}{}",
                     t.name(),
                     start.elapsed().as_secs(),
-                    e
+                    e.display_chain().to_string().trim_right(),
+                    additional_info,
                 );
             }
         }

ci/svd2rust-regress/src/svd_test.rs

@@ -1,10 +1,10 @@
 use errors::*;
-use std::io::prelude::*;
+use reqwest;
 use std::fs::{remove_dir_all, File, OpenOptions};
+use std::io::prelude::*;
+use std::path::PathBuf;
 use std::process::{Command, Output};
 use tests::TestCase;
-use reqwest;
-use std::path::PathBuf;
 
 static CRATES_ALL: &[&str] = &["bare-metal = \"0.1.0\"", "vcell = \"0.1.0\""];
 static CRATES_MSP430: &[&str] = &["msp430 = \"0.1.0\""];
@@ -50,25 +50,27 @@ impl CommandHelper for Output {
         stdout: Option<&PathBuf>,
         stderr: Option<&PathBuf>,
     ) -> Result<()> {
-        if cant_fail && !self.status.success() {
-            bail!(format!("Process Failed! - {}", name))
-        }
-
         if let Some(out) = stdout {
             let out_payload = String::from_utf8_lossy(&self.stdout);
             file_helper(&out_payload, out)?;
-        }
+        };
 
         if let Some(err) = stderr {
             let err_payload = String::from_utf8_lossy(&self.stderr);
             file_helper(&err_payload, err)?;
+        };
+
+        if cant_fail && !self.status.success() {
+            return Err(
+                ErrorKind::ProcessFailed(name.into(), stdout.cloned(), stderr.cloned()).into(),
+            );
         }
 
         Ok(())
     }
 }
 
-pub fn test(t: &TestCase, bin_path: &PathBuf) -> Result<()> {
+pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>) -> Result<()> {
     let user = match ::std::env::var("USER") {
         Ok(val) => val,
         Err(_) => "rusttester".into(),
@@ -134,11 +136,10 @@ pub fn test(t: &TestCase, bin_path: &PathBuf) -> Result<()> {
         &Msp430 => "msp430",
         &RiscV => "riscv",
     };
+
     Command::new(bin_path)
-        .arg("-i")
-        .arg(&chip_svd)
-        .arg("--target")
-        .arg(target)
+        .args(&["-i", &chip_svd])
+        .args(&["--target", &target])
         .current_dir(&chip_dir)
         .output()
         .chain_err(|| "failed to execute process")?
@@ -149,16 +150,15 @@ pub fn test(t: &TestCase, bin_path: &PathBuf) -> Result<()> {
             Some(&svd2rust_err_file),
         )?;
 
-    // TODO: rustfmt < 0.4.0 seems to mangle some doc comments. Omit until this is fixed
-    // Run `cargo fmt`, capturing stderr to a log file
-    // let cargo_fmt_err_file = path_helper_base(&chip_dir, &["cargo-fmt.err.log"]);
-    // Command::new("cargo")
-    //     .arg("fmt")
-    //     .current_dir(&chip_dir)
-    //     .output()
-    //     .chain_err(|| "failed to format")?
-    //     .capture_outputs(false, "cargo fmt", None, Some(&cargo_fmt_err_file))?;
-
+    if let Some(rustfmt_bin_path) = rustfmt_bin_path {
+        // Run `cargo fmt`, capturing stderr to a log file
+        let fmt_err_file = path_helper_base(&chip_dir, &["rustfmt.err.log"]);
+        Command::new(rustfmt_bin_path)
+            .arg(lib_rs_file)
+            .output()
+            .chain_err(|| "failed to format")?
+            .capture_outputs(false, "rustfmt", None, Some(&fmt_err_file))?;
+    }
     // Run `cargo check`, capturing stderr to a log file
     let cargo_check_err_file = path_helper_base(&chip_dir, &["cargo-check.err.log"]);
     Command::new("cargo")

ci/svd2rust-regress/src/tests.rs

@@ -76,7 +76,7 @@ use self::RunWhen::*;
 /// that are not valid in Rust ident
 const BLACKLIST_CHARS: &'static [char] = &['(', ')', '[', ']'];
 
-/// Lovingly stolen from `svd2rust`. Probably could be `Cow`, but I'm lazy
+/// Lovingly stolen from `svd2rust`. Probably could be `Cow`
 pub trait ToSanitizedSnakeCase {
     fn to_sanitized_snake_case(&self) -> String;
 }