Seeking advice on idiomatic testing practices

Hi, I wrote the method below as part of a command line interface app I'm working on.

pub fn get_output_file(output: Option<&str>) -> Result<Box<dyn Write>, Error> {
  match output {
    Some(ref path) => {
      if Path::new(path).exists() {
        remove_file(path)?;
      }
      OpenOptions::new().write(true).create_new(true).open(path).map(|f| Box::new(f) as Box<dyn Write>)
    },
    None => Ok(Box::new(io::stdout())),
  }
}

The Idea is to either open a file or default to stdout the return value is Result<Box<dyn Write>, Error> so it can be used with eg. write! or writeln! macro. When it comes to testing this I start getting confused; Ideally each possible outcome should be tested, so for example I should write a test to verify that a calls to get_output_file(None) should return Ok(Box::new(io::stdout())) and so on, but I feel like I'm kind of loosing this level of detail by normalising the return type to Box<dyn Write> . Do you have any suggestions on how to test/make more testable this method? Would it be acceptable to only rely on integration tests?

I wouldn't bother testing the none input specifically, as that's not a likely place for a bug, and. I'd be more concerned about the race condition between removing and creating the file.

If you really want to test the None input case, close stdout and verify that you get an error writing to the output of this function.

I think the more idiomatic way of doing this would be to completely reverse the graph and do something like :

/// very simplified example

fn output_to_writer<W>(writer: W, data: &Data) -> std::io::Result<()>
where
    W: std::io::Write,
{
    // your writing logic
    writeln!(writer, "{}", data.display());
    Ok(())
}

fn main() -> Result<(), Box<dyn core::error::Error>> {
    let path = std::env_os::nth(1);
    let data = get_data()?;
    match path {
        Some(path) => output_to_writer(File::create(path)?, &data)?,
        None => output_to_writer(std::io::stdout().lock(), &data)?,
    }
    Ok(())
}

Push back up the side-effecting functions instead of putting them at the bottom your stack. You would then be able to test your display logic on any writer and get back a string. (that's basically what the Debug and Display traits allow you to do for types)

2 Likes

Thanks! That is a very interesting point of view. I ended up following a suggestion I got on reddit.
The idea is to define an enum and then implement the Write trait for it, in this way OutputFile can be used as a generic Write interface and can be also tested properly.

    #[derive(Debug)]
    pub enum OutputFile {
      File(File),
      Stdout(Stdout)
    }

    impl Write for OutputFile {
        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
          match self {
            OutputFile::File(f) => f.write(buf),
            OutputFile::Stdout(s ) =>  s.write(buf)
          }
        }

        fn flush(&mut self) -> io::Result<()> {
          match self {
            OutputFile::File(f) => f.flush(),
            OutputFile::Stdout(s ) =>  s.flush()
          }
       }
    }

    pub fn get_output_file(output: Option<&str>) -> Result<OutputFile> {
      match output {
        Some(path) => Ok(OutputFile::File(File::create(path)?)),
        None => Ok(OutputFile::Stdout(io::stdout())),
      }
    }

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

      #[test]
      fn get_output_file_creates_the_file_specified() -> Result<()> {
        let file_path_only = NamedTempFile::new()?;
        let path = file_path_only.into_temp_path();

        let output_file = get_output_file(Some(path.to_str().context("Invalid path")?))?;

        assert!(matches!(output_file, OutputFile::File(_)));
        Ok(())
      }

     #[test]
     fn get_output_file_returns_stdout_when_no_file_is_specified() -> Result<()> {
       let output_file = get_output_file(None)?;

       assert!(matches!(output_file, OutputFile::Stdout(_)));
       Ok(())
     }

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.