bhb / expound

Human-optimized error messages for clojure.spec
Eclipse Public License 1.0
924 stars 24 forks source link

Use Specter instead of postwalk in lift-singleton-groups #206

Closed eneroth closed 3 years ago

eneroth commented 3 years ago

As per discussion in https://github.com/bhb/expound/issues/205

When calling for example (expound/expound some? (d/db conn)) using dev-local, postwalk will throw, due to datomic.core.db.Db implementing the map interface, thereby tricking postwalk into thinking that it can walk through the DB like any map (it can't).

Specter is not so easily tricked by map-like objects, and will not throw when used in place of postwalk.

This PR implements lift-singleton-groups using Specter instead of postwalk.

bhb commented 3 years ago

Is it possible to add a test? I don’t mind having Datomic as a test dependency here. Or perhaps it’s possible to create a DB-like data structure in tests that fails in a similar way?

eneroth commented 3 years ago

Is it possible to add a test? I don’t mind having Datomic as a test dependency here. Or perhaps it’s possible to create a DB-like data structure in tests that fails in a similar way?

That's a good point! It could certainly be done with dev-local, which is free to use, but requires adding some config to Maven. If you're happy with a test that uses it, I could add that.

"Mocking" a failing object should be possible, but I've no idea how to do it at the moment.

bhb commented 3 years ago

That's a good point! It could certainly be done with dev-local, which is free to use, but requires adding some config to Maven. If you're happy with a test that uses it, I could add that.

That would be great, thanks! If it turns into a hassle, don’t worry about it. Thanks for the PR!

bhb commented 3 years ago

Thanks for this patch! I've pulled it into https://github.com/bhb/expound/pull/208 and merged it into my branch for the next release.

avocade commented 3 years ago

There seems to still be a crash issue when a spec fails and datomic db is involved.

This time it's the highlight-value fn (in printer,) which fails in a similar way as this previous issue (Datom can not be cast to Number). It also uses walk, which was also the issue before.