go-cmp

go-cmp copied to clipboard

For larger structs and when the API is potentially evolving it would be handy to have the inverse of IgnoreFields, i.e. cmpopts.FilterFields(typ interface{}, names ...string) cmp.Option.

And for symmetry I guess there should also be FilterInterfaces and FilterTypes.

An alternative naming could be Include… if this clashes to much with the root packages concept of filters.

#53 proposed adding IgnoreUnsetFields for a similar use-case, but I rejected my own proposal. It's possible that we can add IgnoreFieldsExcept.

However, I would like to think through more carefully how we can make filters composable.

It looks like filterField that private and unused now could be a part of solution of this problem with something like IncludeFilter.

I could really use something like this or #53. I have a struct with a bunch of fields and I only want to compare, say, 3-5 of them.

I got something that works for one simple use case by doing this:

func cmpFields(fields ...string) cmp.Option {
	filter := func(p cmp.Path) bool {
		s := p.String()
		if s == "" {
			return false
		}
		for _, f := range fields {
			if s == f {
				return false
			}
		}
		return true
	}
	return cmp.FilterPath(filter, cmp.Ignore())
}

but I'm guessing that there are lots of corner cases that this doesn't handle judging by how much more complicated the cmpopts.IgnoreFields code is. (I haven't had a chance to study it closely yet.)

Suppose we had IgnoreFieldsExcept, what is the user expectation when they see something like the following?

cmp.Diff(x, y, cmp.Options{
    cmpopts.IgnoreFieldsExcept(MyStruct{}, "FooField"),
    cmpopts.IgnoreFieldsExcept(MyStruct{}, "BarField"),
})

I may be mistaken, but I suspect many users will expect that either MyStruct.FooField or MyStruct.BarField will be compared, and that everything else is ignored. However, what actually happens is that no field ends up being compared (since the intersection of two disjoint sets is the null set). Will such a behavior be surprising to users?

How about Include* rather than Ignore*Except and make the default AND not OR to match Ignore* if that's how IgnoreFields functions currently? That's what I'd expect (granted I've never tried to pass multiple IgnoreFields).

// Both FooField and BarField are ignored.
cmp.Diff(x, y, cmp.Options{
    cmpopts.IgnoreFields(MyStruct{}, "FooField"),
    cmpopts.IgnoreFields(MyStruct{}, "BarField"),
})

// Only FooField and BarField are included.
cmp.Diff(x, y, cmp.Options{
    cmpopts.IncludeFields(MyStruct{}, "FooField"),
    cmpopts.IncludeFields(MyStruct{}, "BarField"),
})

I've been following this ticket for a while but I think my use case at the time was I wanted an easy way to diff only fields given by a field mask.

I'm not sure how that'd be implemented. The options framework of cmp assumes that everything is included except for the things that are explicitly ignored. The implementation of cmpopts.IncludeFields would still need to be implemented under the hood in terms of cmp.Ignore options (which would lead to the possible composability thorn mentioned above). We could introduce another primitive option called cmp.Include, but that would open yet another can of worms, such as what happens when both a cmp.Ignore and a cmp.Include is applicable for the same node.

I should further note that cmpopts.IncludeFields would still imply an implicit ignore of all other fields except the ones listed. The fact that some fields are being ignored seems like a mismatch with it's name.

@dsnet

I may be mistaken, but I suspect many users will expect that either MyStruct.FooField or MyStruct.BarField will be compared, and that everything else is ignored. However, what actually happens is that no field ends up being compared (since the intersection of two disjoint sets is the null set). Will such a behavior be surprising to users?

Personally the behavior is what I'd expect, though perhaps others would be confused.

I can see what you're angling at in wanting options to be coherently composable, but in practice I want to compare a limited subset of struct fields pretty often and I've never once composed multiple ignore options. I think there's a case for being pragmatic here.

Perhaps a note warning the reader about stacking ignore options together on the doc comment for IgnoreFieldsExcept would be sufficient.

Or, consider exporting more primitives in cmpopts so I can build this (and #53) myself outside of cmp/cmpopts without copy-pasting large swaths of struct_filter.go.

\cc @neild, do you have an opinion about whether to add IgnoreFieldsExcept?

I personally have also needed something like IgnoreFieldsExcept a few times. My fears about users being surprised by composability of this option may be entirely unfounded.

Or, consider exporting more primitives in cmpopts so I can build this (and #53) myself outside of cmp/cmpopts without copy-pasting large swaths of struct_filter.go.

I've been wanting to export filterField for a long time, but the composability questions around that are even more complex than simply the composability of IgnoreFieldsExcept. @rogpeppe and I had a relatively long discussion on Slack about it. I don't think we came to any clear consensus on what to do.

I've been wanting to export exportField for a long time,

(Do you mean filterField?)

Yep, fixed.

I don't think the composability of IgnoreFieldsExcept is particularly surprising: It isn't hard to understand that the fields which remain are the union of all fields that were not ignored.

Hello! Any updates on this? I'm interested because we have structs with many fields and specifying fields to be ignored makes the tests less readable. I can contribute too if IgnoreFieldsExcept has been agreed on. Thanks!

I am also interested in these feature. I am looking to see if I a struct contains expected fields (which only includes the values of the field I care about, while other fields contain dynamic values) in my tests.

I am interested in this feature too, would be really helpful in comparing only the required fields. Any timelines when this might be expected to be taken up?

Any update? whitelist is very useful for testing.

I am interested in this feature too.

Check this out.

func IgnoreAllUnexported() cmp.Option {                  
    return cmp.FilterPath(func(p cmp.Path) bool {        
        sf, ok := p.Index(-1).(cmp.StructField)          
        return ok && unicode.IsLower(rune(sf.Name()[0])) 
    }, cmp.Ignore())                                     
}