Stream: wg-traits

Topic: chalk tests on nightly chalk#220


nikomatsakis (May 03 2019 at 19:59, on Zulip):

BTW, a good issue for someone who wants to hack on chalk quickly is to fix the nightly tests :)

My suggestion would be to strip trailing commas, but we can also just edit the tests:

https://github.com/rust-lang/chalk/issues/220

matklad (May 03 2019 at 20:00, on Zulip):

We have exactly the same problem in rust-analyzer :D

nikomatsakis (May 03 2019 at 20:00, on Zulip):

lol

nikomatsakis (May 03 2019 at 20:00, on Zulip):

breaking change!

nikomatsakis (May 03 2019 at 20:00, on Zulip):

ps @centril clearly people do rely on Debug impl output ;)

matklad (May 03 2019 at 20:01, on Zulip):

obligatory spacebar heater xkcd link

nikomatsakis (May 03 2019 at 20:01, on Zulip):

lol

nikomatsakis (May 03 2019 at 20:01, on Zulip):

yes

nikomatsakis (May 03 2019 at 20:01, on Zulip):

exactly

centril (May 03 2019 at 20:01, on Zulip):

@nikomatsakis yes, it happens sometimes, annoying

nikomatsakis (May 03 2019 at 20:01, on Zulip):

Actually, I think it'd be a nice library to put on crates.io to do the comparison

nikomatsakis (May 03 2019 at 20:01, on Zulip):

it could do normalization of this kind etc

matklad (May 03 2019 at 20:02, on Zulip):

@nikomatsakis https://github.com/mitsuhiko/insta/ exists

nikomatsakis (May 03 2019 at 20:02, on Zulip):

of course it does

nikomatsakis (May 03 2019 at 20:02, on Zulip):

(maybe we should port to that then...)

nikomatsakis (May 03 2019 at 20:02, on Zulip):

gotta run

centril (May 03 2019 at 20:02, on Zulip):

we should use property based testing more :P

centril (May 03 2019 at 20:02, on Zulip):

//shameless plug for proptest

detrumi (May 04 2019 at 07:06, on Zulip):

insta doesn't handle those differences, though I suppose you might be able to use insta's redactions for that

detrumi (May 04 2019 at 07:14, on Zulip):

Oh wait, seems like you can't use redactions in the assert_debug_snapshot_matches (only snapshots based on serde::Serialize support redactions)

detrumi (May 04 2019 at 10:49, on Zulip):

Created #222 to strip the commas, but there's still one failing test because of a change in debug formatting where the order changed

nikomatsakis (May 04 2019 at 10:51, on Zulip):

Curious. I guess it's a hashset.

nikomatsakis (May 04 2019 at 10:52, on Zulip):

We could probably override Debug and sort the keys ourselves or something to ensure a consistent order, independent of the underlying Debug impl.

nikomatsakis (May 04 2019 at 10:52, on Zulip):

(I wuld probably do this by creating a vec, sorting the vec, and then invoking the vector's debug impl)

detrumi (May 04 2019 at 11:48, on Zulip):

@nikomatsakis That would work if DelayedLiteral implemented Ord, but looking at the hand-written Eq impls, it seems like we'd prefer to avoid that?

detrumi (May 04 2019 at 11:57, on Zulip):

Or we could sort the debug output instead... hmm

matklad (May 04 2019 at 12:12, on Zulip):

Another alternative is to just fix the seed in the hash-map. Not protecting from DoS seems fine for chalk? And consistent ordering also could help in printfdebuggin

detrumi (May 04 2019 at 13:15, on Zulip):

@matklad That doesn't necessarily fix it though, since even with a fixed seed the implementation might've changed between stable and nightly

matklad (May 04 2019 at 13:15, on Zulip):

Ah, riiiight, it's not because of the seed, it's because of hashbrown

detrumi (May 04 2019 at 14:19, on Zulip):

Alright, ended up just writing out a debug impl for DelayedLiteralSet that sorts by debug output. It's a bit of a hack, but it fixes the tests :slight_smile:

Last update: Nov 12 2019 at 15:30UTC