assert-rs / predicates-rs

Boolean-valued predicate functions in Rust
docs.rs/predicates
Apache License 2.0
173 stars 29 forks source link

WIP: Change diff crate #102

Closed vincentdephily closed 3 years ago

vincentdephily commented 3 years ago

This explores a fix for #94.

There are two replacement crates in this PR, but it only makes sense to merge one. Neither crate can be used as an exact replacement of the unmaintained crate, because the diffing algorithms don't match : the calculated "distance" is different and there is no way to configure a "split character".

Nevertheless, I used the two crates to showcase two possibilities:

Both crates have their own feature flag, and I resisted bikeshedding the name of the constructors.

IMHO the second one is more tempting, except if we want to avoid tying ourselves into a particular diffing crate and want to to the most common denominator features.

We could make a release that deprecates the difference feature and adds the similar feature, or we could make an immediate replacement (simplifying the bikeshedding and cosing the rustsec advisory faster).

TODO:

vincentdephily commented 3 years ago

Looks like easy CI fixes, I'll do that during the week.

Interestingly, looks like pretty-assertions selected diff as a replacement after looking at similar. For some reason I hadn't looked at diff, not sure if it's any better for our usecase.

asomers commented 3 years ago

Which predicates are changing exactly? Is it just similar or are there others too? I would say that you should do what you have to, and I'll note it in Mockall's CHANGELOG .

epage commented 3 years ago

Which predicates are changing exactly? Is it just similar or are there others too? I would say that you should do what you have to, and I'll note it in Mockall's CHANGELOG .

We might be changing the settings for the `DifferencePredicate which is created via predicates::str::similar. This would require a major version bump for predicates (but not predicates-core where the Traits live) and any crate that re-exports predicates would need to do similar.

If we do a breaking change, my question is if we need either of these functions (choose the split, allow edit distance besides 0)

vincentdephily commented 3 years ago

We might be changing the settings for the DifferencePredicate which is created via predicates::str::similar.

Also, even if we keep the .distance() API intact, the measured distance value will no longer be the same. So if a user has a test with a fine-tuned distance value, it's likely no longer correct and implies a major version bump too.

epage commented 3 years ago

Thanks again for all of this research. I've pulled it together in #107. I ended up using a different crate just because another of my crates was using it. We can always swap out and add features as there is demand.