rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Check for possible silently passing invalid tests by making sure some sort of `panic!` exists in tests

Open kayagokalp opened this issue 1 year ago • 2 comments

What it does

Consider the following code piece:

...
#[test]
fn test_with_silent_fail() {
  let result = ...;
  
  if result.is_err() {
     eprintln!("failed to do something");
  }
}

This is recently discovered for a timout use case, the test had fail cases correctly implemented but also had a outer check for timeouts with tokio runtime. If the user is using eprintln! inside a test, I feel like the test should also fail in that block. Otherwise println! should be used instead.

So in the control graph, the block that contains eprintln should contain a panic as well, or eprintln should be a println in my opinion and clippy can enforce this.

Advantage

It helps preventing cases where a test can silently pass, while the intention was to give an error thus fail.

Drawbacks

It might hurt the expressivity a little, as people may want to use eprintln inside a test for other reasons than printing an error.

Example

#[test]
fn test_with_silent_fail() {
  let result = ...;
  
  if result.is_err() {
     eprintln!("failed to do something");
  }
}

Could be written as:

#[test]
fn test_with_silent_fail() {
  let result = ...;
  
  if result.is_err() {
     panic!("failed to do something");
  }
}

or

#[test]
fn test_with_silent_fail() {
  let result = ...;
  
  if result.is_err() {
     println!("failed to do something");
  }
}

kayagokalp avatar Mar 14 '24 01:03 kayagokalp