Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive PartialEq for Error #98

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joecorcoran
Copy link

@joecorcoran joecorcoran commented Feb 21, 2020

This helps when wrapping this crate's Error struct in another crate. Not sure if you'd like a test to cover this too, but if so just let me know.

This helps when wrapping this crate's Error struct in another
crate.
@sfackler
Copy link
Owner

I don't know if comparing the string description of the underlying error is the best idea.

@joecorcoran
Copy link
Author

Is there an approach that you prefer that I can help with? I guess implemeting PartialEq is also a way forward. Right now I'm struggling to wrap this crate's error type and wondering how to move forward without resorting to converting the error to a String.

@sfackler
Copy link
Owner

Why do you need to compare your wrapped error for equality? There are a lot of error types in the wild that cannot do that (e.g. io::Error, Box<dyn Error>, ...).

@joecorcoran
Copy link
Author

Yeah I know the problem with io::Error exists too. I want to use assert_eq! when writing library tests to demonstrate that when the lib is used in a certain way, a particular type of error occurs, not just any error. If even one of the wrapped error types can't be compared, the whole enum can't be compared and thus the whole Result<_, Err> can't be compared. The bad ergonomics bubble up through the codebase.

As I say above, ultimately there are ways for me to work around this, but since the error type in this crate is fairly simple and doesn't use Box<dyn ... etc. I thought I'd try this PR out. Your call.

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

Successfully merging this pull request may close these issues.

None yet

2 participants