lambdaisland / hiccup

Enlive-backed Hiccup implementation (clj-only)
Mozilla Public License 2.0
28 stars 4 forks source link

Add tests from Reagent's test suite #6

Open alysbrooks opened 1 year ago

alysbrooks commented 1 year ago

Since we're going for Reagent compatibility, it makes sense to leverage their test suite.

https://github.com/reagent-project/reagent/blob/master/src/reagent/impl/template.cljs

Follow up of #4 and #5.

alysbrooks commented 1 year ago

@rafd I'm planning to address this issue, but if you'd be interested, let me know.

rafd commented 1 year ago

@alysbrooks I added attribute conversion tests for my patch, which test the behaviour of Reagent plus React (b/c React also does additional stuff on top of Reagent).

I don't have bandwidth atm to integrate the Reagent tests, so please go ahead.

alysbrooks commented 1 year ago

@rafd No problem, just thought I'd offer.

alysbrooks commented 11 months ago

Not sure where this falls on our priorities. It would be more of an effort to pre-emptively find gaps or bugs than addressing a specific need. On the other hand, it would be a relatively cheap way to increase our confidence we are compatible with Reagent.

One way to look at it is, if we are already compatible, incorporating these tests will probably go relatively quickly since they'll pass. If we aren't, it will take longer, but that means we've found actual bugs. When I worked on #4 there were some edge cases to consider, but it went relatively quickly, and I was able to document those differences. The main delay was waiting for another PR to merge (and then having to rebase it, which I have yet to do).