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

Unexpected regression with 1.2.0 #95

Open
mre opened this issue Mar 18, 2022 · 3 comments
Open

Unexpected regression with 1.2.0 #95

mre opened this issue Mar 18, 2022 · 3 comments

Comments

@mre
Copy link

mre commented Mar 18, 2022

We use rust-pretty-assertions for lychee.
There is a PR for upgrading to 1.2.0 here, which fails because of the changes in #92.
Previously, using assert_eq on two HashSets would work, but it fails now because of the missing AsRef<str> impl.

Before, the assertion was

assert_eq!(uris_html5gum, uris_html5ever);

and I could use a workaround like this

assert_eq!(format!("{uris_html5gum:?}"), format!("{uris_html5ever:?}"));

but I wonder if I'm missing something. The PR mentioned an assert_debug_eq that would behave like the old version. Is this planned?

@tommilligan
Copy link
Collaborator

Hi, thanks for reporting this. This certainly looks like a bug that you have an upgrade failing with a compile error - I will take a look this weekend and ping you once I know more.

@tommilligan
Copy link
Collaborator

Just to update you - this is definitely a bug, and I think I can see why it happens. I have a couple of avenues of investigation to persue, but I've been sick for a couple of days this week so probably won't make any progress until next weekend.

If you have the time, a minimally failing test case using standard library types would be very useful. I can see your specific test fails, but haven't been able add a minimally failing example to the library's test suite yet. If you're able to add one, it should live here: https://github.com/colin-kiegel/rust-pretty-assertions/blob/9654759b57a88cec10fd4ffac563a93a80006689/pretty_assertions/tests/macros.rs#L29


It looks like your case is trying to use the wrong impl block out of:

and the compiler is correctly pointing out that your types do not implement CompareAsStrByDefault, which was introduced in #92.

This feels more generally like the problem of implementing mutually exclusive trait implementation for types, where trait implementations should always be additive 🙁 In general, it's not correct to say "and for all other types, use this other implementation", which is what #92 attempts by using a different tuple type, which can be automatically converted to (deref'd?) by the compiler.

@mre
Copy link
Author

mre commented Mar 23, 2022

Thanks for looking into this and there is certainly no rush on our end to resolve this. The pervious version works fine still and we have a workaround for the the latest version.
I will try to add a test case for it if I find the time, but if anyone else is faster, feel free to give it a shot. 😉

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

No branches or pull requests

2 participants