add DeepEqual and start using it in tests

[mvdan]

(see commit message)

[mvdan]

@warpfork this is my take on #173, needed for the schema/gen/go test refactor.

Once you review, I'll add tests and write a good commit message, and then we can merge.

Worth noting that the quality of the test checks went down when nodes aren't equal:

        --- FAIL: TestStructNesting/bhvtest/repr-create (0.00s)
            testStruct_test.go:136: ShouldEqual check rejected:
                          bool(
                        -       false,
                        +       true,
                          )

It no longer diffs, so it just tells us yes/no. However, we could easily improve this with https://pkg.go.dev/github.com/google/go-cmp/cmp#Comparer, since our DeepEqual does match the shape func(T, T) bool.

However, I could not get this to work with go-wish via Wish(t, n, ShouldEqual, nr, cmp.Comparer(ipld.DeepEqual)):

schema/gen/go/testLists_test.go:62:45: cannot use cmp.Comparer(ipld.DeepEqual) (type cmp.Option) as type wish.options in argument to wish.Wish:
        cmp.Option does not implement wish.options (missing wish._options method)
schema/gen/go/testLists_test.go:97:45: cannot use cmp.Comparer(ipld.DeepEqual) (type cmp.Option) as type wish.options in argument to wish.Wish:
        cmp.Option does not implement wish.options (missing wish._options method)

So I think we could do that if/when we move to quicktest, which definitely supports cmp options: https://pkg.go.dev/github.com/frankban/quicktest#CmpEquals

[warpfork]

Mostly solid except I think we'll need to do something about links. If links always return false, that'll make a lot of blocks turn false, and it won't be very useful.

[warpfork]

I think we might skip this mention. It's probably true, but there's nothing particularly regrettable about this function that means we need to discourage its use, as far as I can figure.

[mvdan]

The panics :)

[warpfork]

Still.

We can document that if a node breaks contract badly (e.g. fails to answer AsString after saying Kind()==String) it panics... but that requirement is sufficiently baseline that I think it's acceptable.

[rvagg]

Since there are lots of places where you're having to panic() would it be worth just returning (error, bool)? or is it really important to follow the reflect.DeepEqual pattern here? If you did that then it'd at least not have to come with caveats and could be used for legitimate runtime purposes (I could imagine up a bunch of ways this would be really useful outside of debugging).

[mvdan]

The thing with returning an error is that it can only happen if a Node implementation breaks the interface contract. For example, if calling AsInt on a Kind==Int node errors.

So those errors should be truly rare and unexpected, unless I'm missing something. Panic seems fine in that way.

Arguably, the interface could have been designed in a way that errors are not part of this equation, and one simply cannot call AsString on an integer kind node. One could do this with optional interfaces, for example, so that AsString is only implemented by nodes which support it, and it does not return an error.

Maybe ADLs would still need an error? I'm still not super clear on that. If we think DeepEqual might be used for ADLs, and it's good for them to return errors like these, then maybe we should make this return an error after all. Though, for the average use case, the error would always be nil.

[warpfork]

I'm good with this being merged as soon as the links get handled :)

[mvdan]

This now has a proper commit message and its own tests. I've also implemented links as discussed. Needs a final review :)