Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Improving error matching consistency (at least for testing) #14

Open
U007D opened this issue Nov 4, 2017 · 5 comments
Open

Improving error matching consistency (at least for testing) #14

U007D opened this issue Nov 4, 2017 · 5 comments

Comments

@U007D
Copy link
Contributor

U007D commented Nov 4, 2017

TL;DR

Can Failure introduce a consistent interface that implements PartialEq across all Fail types so errors can be compared for equality simply and literally? This will improve the ergonomics and productivity around unit testing, and possibly other scenarios as well.

Motivation

As a BDD developer, the first thing I do when developing new code is to write a unit_test. Rust's lack of consistent API around errors makes writing unit tests less ergonomic than perhaps they should be. Here's a simple example of what I would like to write (which works, but only for some error types(!)):

#[test]
fn passing_bad_param_results_in_not_found_error() {
    assert_eq!(some_func(bad_param).err().kind(), std::io::ErrorKind::NotFound);
}

I've run into what I consider to be major ergonomic issues with errors and unit testing. I haven't found a consistent way to work with errors that doesn't involve a lot of pattern-matching boilerplate. Here are some details adapted from a posting I made about a month ago:

No consistent basis with which to work with Errors

(inconsistencies or issues in bold)

using the Standard Library

  • e.g. std::io::Error:

    • is a struct encapsulating an enum
    • does not implement PartialEq
    • does implement a kind() method
    • does implements ErrorKind variants which implement PartialEq
  • e.g. std::fmt::Error:

    • is a (marker) struct
    • does implement PartialEq (among other traits)
    • does not implement a kind() method
    • does not define ErrorKind variants
  • e.g. std::io::CharsError

    • is an enum
    • does not implement PartialEq
    • does not implement a kind() method
    • does not define ErrorKind variants
  • std::error::Error

    • is a trait
    • does not mandate/require errors implement PartialEq
    • does not mandate/require a kind() method
    • does hot mandate/require ErrorKind variants

using error-chain

  • error-chain-created errors
    • do not implement PartialEq
    • do implement a kind() method
    • do implement ErrorKind variants which do not implement PartialEq

using pattern matching

  • pattern-matching (as opposed to via PartialEq) is often cited as the recommended way to distinguish Errors, given these numerous inconsistencies). There are crates (such as matches) for reducing boilerplate when pattern-matching
  • unfortunately, pattern matching Errors adds boilerplate, obfuscates code and is an incomplete solution.

For example:

fn string_validator_ref_handles_empty_input() {
  // GIVEN an empty string
  let input = String::new();
  let expected_result = ErrorKind::ValueNone;

  // WHEN some operation which generates an error occurs
  let result = (&input).validate_ref::<NonEmptyStringValidator>().err().unwrap()
/* What I would *like* to be able to write */

// THEN ensure the expected error was returned
  assert_eq!(*result, expected_result);
}
/* The reality */

// THEN ensure the expected error was returned
  assert!(matches!(*result, expected_result); //Error: pattern unreachable (match doesn't work this way; unintuitive at this level)
  assert!(matches!(*result, val if val == expected_result)); //not ergonomic; Error without PartialEq (e.g. error-chain): binary operation `==` cannot be applied to type `error::ErrorKind`
  assert!(matches!(*result, ErrorKind::ValueNone); //works, but without variable support, important scenarios become unrepresentable:
      // suppose two methods must yield same result.  To ensure consistency through refactorings, a test is developed:
      // assert_eq!(matches(FuncA(badInput), FuncB(badInput)) //not possible, as per above; only constant variants w/PartialEq implemented can be used ergonomically

When considering Error equality, I believe it is out of scope for to consider the semantics of errors (e.g. "Are two instances of an error's Other variant equal?" Assuming the two instances of Other are value-equal then, yes. The fact that Other could be used by a library or application as a 'catch-all' to represent a wide variety of different error conditions in the real world is out of scope for this proposal. It is expected that the user of that error type will understand when a comparison of Other is and is not a meaningful operation).

Given that the Error type is freqently implemented using different types (marker structs, encapsulating structs, enums, and others) the only dependencies we should take are on its API. To that end, I would love to see a consistent .kind() method (or something else which serves the purpose of providing the error variant) which exists and is properly defined for every Failure.

@U007D U007D changed the title Improving consistency for testing Improving error matching consistency (at least for testing) Nov 4, 2017
@withoutboats
Copy link
Contributor

I would like for error types to implement PartialEq, and for Error to, but there are problems. The core issue issue is that PartialEq is not object safe. This means we can't use it as a supertrait on Fail, and we can't really implement it for Error, which has a trait object inside it.

I have a vision of a solution, which requires specialization (an unstable feature). With specialization, we could support generating an (unsafe) function pointer for PartialEq if the type implements PartialEq, and not otherwise. Then, if the type ids match & there is a PartialEq implementation, we downcast both errors and dynamically call the PartialEq impl. (This conforms to PartialEq because PartialEq does not require reflexivity.)

In that sort of solution, we wouldn't require PartialEq, but Errors can be compared if their Fail type does implement it.

There's also nothing to stop users from deriving PartialEq for their Fail types now, whereas error-chain does not allow you to implement PartialEq for your error type at all.

@U007D
Copy link
Contributor Author

U007D commented Nov 5, 2017

Thanks for this, @withoutboats. I have a couple of thoughts, but want to spend a few days to ramp up on object safety first to know how viable my ideas might be before posting.

@U007D
Copy link
Contributor Author

U007D commented Nov 16, 2017

I've been delving into the concept of object safety, its reason for being, history, etc. I can't say I understand it all, and have many questions about why some limitations haven't been eliminated by compile-time trait object plumbing, but these questions are far outside the scope of this issue.

So for this issue, even "dreaded" (from a matching consistency perspective) std::io::Error's discriminants ultimately boil down to either an i32 (for the Os variant) or a std::io::ErrorKind (for the Simple and Custom variants), both of which implement PartialEq. Simpler error definitions such as std::fmt::Error and std::str::Utf8Error, are enums in their own right, which, AFAICT, all seem to directly derive PartialEq.

So my latest thinking is that without plugging holes in what types of traits can be considered object safe traits, whether Fail::Error could have a .PartialEqErrorValueUsableForConsistentComparison() method (the name is a work in progress ;)) which would return the error type's (PartialEq) discriminant so that the user could then at least compair Fail::Error's consistently (perhaps a la std::mem::Discriminant or possibly HKT could help? Note, std::mem::Discriminant is actually generic, so yay, more object safety concerns, but not sure how much of an issue that is for Fail::Error).

@shepmaster
Copy link

@U007D FWIW, PartialEq is the reason I use quick-error.

@U007D
Copy link
Contributor Author

U007D commented Nov 17, 2017

@shepmaster although better than the then-alternatives, I found quick-error still too verbose... Failure has been superb thus far.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants