Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
MasterMedo opened this issue Nov 27, 2023 · 1 comment
Closed

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

MasterMedo opened this issue Nov 27, 2023 · 1 comment

Comments

@MasterMedo
Copy link

MasterMedo commented Nov 27, 2023

Similar to golang/protobuf#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
Copy link
Author

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.
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant