Idiomatic tests for Config::new from the Rust book

I'm writing tests for the Config::new method as suggested in The Book in 12.04.

Feel free to write some tests for the functionality in the Config::new and run functions on your own.

At that stage in the book the Config struct and new method look like this:

#[derive(Debug, PartialEq)]
pub struct Config {
    pub query: String,
    pub filename: String,
}

impl Config {
    pub fn new(args: &[String]) -> Result<Config, &'static str> {
        if args.len() < 3 {
            return Err("not enough arguments");
        }

        let query = args[1].clone();
        let filename = args[2].clone();

        Ok(Config { query, filename })
    }
}

Config::new is used in main like this:

    let args = env::args().collect::<Vec<String>>();

    let config = Config::new(&args).unwrap_or_else(|err| {
        println!("Problem parsing arguments: {}", err);
        process::exit(1);
    });

My tests for Config::new currently look like this:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_config_all_args() {
        let args = [
            "path/to/binary".to_owned(),
            "query".to_owned(),
            "filename.txt".to_owned(),
        ];
        let conf = Config::new(&args).unwrap();
        assert_eq!(conf.query, "query");
        assert_eq!(conf.filename, "filename.txt");
    }

    #[test]
    fn test_config_insufficient_args() {
        let args = ["binary_name".to_owned()];
        assert_eq!(Config::new(&args), Err("not enough arguments"));
    }
}

Are these tests ok?

The main thing I wondered about was the repeated to_owned to create an array of Strings. Should I care that to_owned() is repeated?

let args = [
    "path/to/binary".to_owned(),
    "query".to_owned(),
    "filename.txt".to_owned(),
];

let args = ["binary_name".to_owned()];

I could do something like "query".into() to reduce line noise a little, but to_owned() or String::from() seem more explicit.

The tests look ok to me. People diagree whether into/to_owned/new() is the right way, but in general if you need a slice of owned objects, then something like this will be needed.

However, the &[String] type is useful only in very limited case where you know you already have Vec<String> that you're going to need later, and only need to give a function a temporary view of it, and don't want to reallocate to do so.

In your case you won't need the Vec later, so you could pass Vec<String> to allow the new() function to pop() elements out of the Vec, and avoid clone().

Otherwise if the new() function is going to clone anyway, it could take &str. Ideally as multiple args, so that the caller doesn't need to make Vec<&str> (it's not possible to cast Vec<String> to Vec<&str>, because String is 3 usize large, and &str is 2 usize large).

1 Like