gstruct should allow for default matching mixed with customized matching.

[flowchartsman]

Currently gstruct allows you to do one of the following

• Match where matchers are provided for everything
• Provide custom matchers for certain fields and either
  • Ignore missing fields
  • Ignore missing matchers

It would be even more useful if there was an additional option to provide custom matchers for certain fields and fall back to reflect.DeepEqual if a matcher isn't specified. This would allow for elegant handling of certain edge-case fiddliness like you sometimes see with time.Time

There are four ways I can see this being handled

1. a new method like MatchAllFieldsWithDefault
2. a new option to MatchFields, AllowDefault
3. a new option to MatchAllFields, Fallback or UseDefault
4. change the behavior of MatchAllFields to use reflect.DeepEqual if a matcher is not provided

Personally, I feel like 4 is the right choice because it seems unnecessary to have an additional error condition for "hey, you didn't provide matchers for everything" when the docs already specify that it requires all fields be matched. It seems like it would be in keeping with the spirit of the library if MatchAllFields matched everything, whether you remembered to set a matcher or not. Following that, 1 is probably the next best choice, as it changes no behavior.

[onsi]

@timstclair wdyt?

[timstclair]

I agree this would be useful, but have an alternative proposal: wildcard field names. The simplest would be to just treat * as the default matcher, but we could eventually expand it to allow more advanced wildcard (or full regex) matching (I don't know how useful that would be). The wildcard would be a little more complicated for the Elements matcher, since there are no restrictions on element names, but maybe that's not a concern.

I don't like option 4, since for my usecase I actually do want it to be an error if matchers weren't provided for all fields. The reason is I want to make sure all fields in a struct have a test, and if another developer adds a new field to the struct, I want to require them to update the test.

[flowchartsman]

Better Proposal

Now that I think about it some more, I'm not sure my understanding of gstruct was entirely correct, and I'm not sure how useful wildcard matchers would be, given your declarative model. I agree they might find some use, but I can't see it being terribly common, and it's orthogonal to what I actually wanted (sorry about that!).

So, with that in mind, here is a more clear proposal:

As it stands, MatchAllFields() and MatchFields() don't deal with any kind of expected at all, and I would like to provide an expected with the option to override certain comparisons that would otherwise use reflect.DeepEqual. Given this description, it seems that a new method is most appropriate, and would look something like the following:

Expect(actual).To(MatchAgainst(expected, Fields{
    "Foo": Ignore(),
    "TimeField": Custom(timeEquals),
    "SomeNumberField": BeNumerically(">", 1), //they don't have to match, this just has to be higher than some value
    "SomeNestedStruct": MatchAgainst(expected.SomeNestedStruct, Fields{
        "Bar": Ignore(),
        //everything else for actual.SomeNestedStruct <-> expected.SomeNestedStruct uses reflect.DeepEqual
    },
    //everything else for actual <-> expected uses reflect.DeepEqual
})

timeEquals := func(actual interface{}, expected interface{}) (bool, error){
    timeActual, ok := actual.(time.Time)
    if !ok { /* ... */ }
    timeExpected, ok := expected.(time.Time)
    if !ok { /* ... */ )
    return timeActual.Equal(timeExpected), nil
}

[timstclair]

Yes, I think you're right about the issue with the wildcard matchers. IIUC, you're looking for something like "deep equal with override". I can see the use of that, your proposal makes sense. Would this only be for struct (fields) matchers? It's a little harder to envision how it would work with a slice (elements) matcher.

[flowchartsman]

I'm not sure, I'd have to dig into it a bit. Conceivably you could have another method that simply doesn't take an expected and returns a closure that you pass the slice element to, but that wouldn't work beyond the first level, really. I feel comfortable saying that, if someone wants to apply MatchAgainst (or whatever you want to call it) to a slice, they'll have to use a for range to get at the individual elements.

[onsi]

Any thoughts on this one y'all? Anyone up for a PR?

[flowchartsman]

I've been meaning to take a crack at it, but I haven't had time. Should have some free time next week to dig in.

[williammartin]

@flowchartsman Seems like you never got around to this, do you still use gomega? Is this still something that would be valuable to you?

[flowchartsman]

I do, but in a much more limited capacity. I think it would be exceptionally useful, but I'm exceptionally busy and would need to reacquaint myself with the code. If anyone is willing to pair up and hack on it some weekend, I'm game.