google / go-cmp

Package for comparing Go values in tests
BSD 3-Clause "New" or "Revised" License
4.08k stars 209 forks source link

cmpopts: add EquateUnsetFields(want) or FilterFieldsFrom(want) #345

Closed MasterMedo closed 7 months ago

MasterMedo commented 7 months ago

Similar to https://github.com/golang/protobuf/issues/1578, this is a feature request to create a function that supports extracting set fields from a struct and defining filterField functions for them.

Is your feature request related to a problem? Please describe. Comparing struct types in full with the cmp.Diff or cmp.Equal makes for a change detector test. This is because every time a new field is added to the struct all old tests need to be updated to have this field as well. Additionally, the lack of this feature causes many side values of the return type to be mocked. The problem with that is that often the mocked values don't contribute to the validity of the test and when they change in the implementation all tests need to be changed as well.

Describe the solution you'd like I'd like an option like protocmp.IngoreUnsetFields(m protobufMessage) to exist. It would be used by passing in the want protobuf message as the argument. E.g.

func TestMessageContent(t *testing.T) {
  content := "content"
  want := MyStruct{
    Content: content,
  }
  got := MyStructFromContent(content)
  if diff := cmp.Diff(want, got, cmpopts.EquateUnsetFields(want)); diff != "" {
    t.Errorf(...)
  }
}

Describe alternatives you've considered Alternatively, one has to manually implement the function by using the cmp.FilterField function.

Additional context This started as a document created internally in Google because often when a field was added to a struct or default values that were mocked in all tests were changed old tests that didn't test the functionality of the changed values needed to be changed. The addition of this function only partially solves the issues in that document, but it's a great first step!

MasterMedo commented 7 months ago

I'm retracting this feature request. Structs are significantly different from Protobuf messages in the sense that the primitive values of structs don't have corresponding HasField functions. That would introduce a challenge in the current proposal because one couldn't distinguish between an unset primitive field and a field set to the default value of the primitive. E.g.

type person struct {
    name string
    age  int
}

func personFromName(name string) person {
  return person{Name: name, Age: 5}
}

func TestPersonName(t *testing.T) {
  name := "John"
  want := person{
    Content: content,
    Age: 0,  // Age would be ignored because a default value can't be distinguished from an unset value
  }
  got := personFromName(name)
  if diff := cmp.Diff(want, got, cmpopts.EquateUnsetFields(want)); diff != "" {
    t.Errorf(...) // Error would not be thrown even though Age 0 and Age 5 are different.
  }
}