Proposal: Global setting of context/size for `GetUnifiedDiffString`

[Radek-Pysny]

Description

1. It would be great to have some global configuration that might affect some feature of testify package. The only one that is truly missing (at least for me) is "customisation" of one numeric magic constant used by your internal diff function (see comment in the snippet below, please).
2. Just a small nit-pick. That function called diff introduces local variable called diff. How about calling it, e.g., result?

// diff returns a diff of both values as long as both are of the same type and
// are a struct, map, slice, array or string. Otherwise it returns an empty string.
func diff(expected interface{}, actual interface{}) string {
	if expected == nil || actual == nil {
		return ""
	}

	et, ek := typeAndKind(expected)
	at, _ := typeAndKind(actual)

	if et != at {
		return ""
	}

	if ek != reflect.Struct && ek != reflect.Map && ek != reflect.Slice && ek != reflect.Array && ek != reflect.String {
		return ""
	}

	var e, a string

	switch et {
	case reflect.TypeOf(""):
		e = reflect.ValueOf(expected).String()
		a = reflect.ValueOf(actual).String()
	case reflect.TypeOf(time.Time{}):
		e = spewConfigStringerEnabled.Sdump(expected)
		a = spewConfigStringerEnabled.Sdump(actual)
	default:
		e = spewConfig.Sdump(expected)
		a = spewConfig.Sdump(actual)
	}

	diff, _ := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
		A:        difflib.SplitLines(e),
		B:        difflib.SplitLines(a),
		FromFile: "Expected",
		FromDate: "",
		ToFile:   "Actual",
		ToDate:   "",
		Context:  3,   // <--- 1. THIS COULD BE CONFIGURABLE
	})

	return "\n\nDiff:\n" + diff  // <--- 2. SHADOWING OWN FUNCTION NAME
}

Proposed solution

No proposed solution yet.

Use case

There are use cases, where just 3 lines (one with error + one above + one below) is not sufficient. When I hit issue and need additional context to get strong enough clue to solve failing presumption, I have to change that magic constant to a higher value to get more context. This is mostly issue with struct within another struct.

[dolmen]

1. 👎 for global configuration. That would not work well with parallel tests.

But the Assertions object could be a good place for storing such settings.

2. Shadowing is not an issue: diff is not recursive.

[Radek-Pysny]

Assertions might be the way. BTW, it does work like assert.XYZ not like require.XYZ, am I right?

[fredbi]

Global configuration is a real footgun and once you’ve introduced it you can never get rid of it.
I’ve been also examining the possibility to hijack the …msgWithArgs to inject options. But again, this just makes the whole thing harder to predict.

The Assertion struct is indeed the right way to hide/reuse settings (either with assert or require).

Overall, there are very few such exported optional settings currently and all can easily be dealt with a plain constant (e.g delta, epsilon).

For internal settings (your example) a possible way is to override the assertions and consume directly from the difflib (i can’t remember if it is internal now or not)

[Radek-Pysny]

I agree. Does it make sense to include this kind of configuration in Assertions types. Question is how to name it a when it might be introduced. Should I contribute by PR?

[dolmen]

Should I contribute by PR?

Definitely NOT. Too early.

Your request seems to fit a more general area of customizing Testify output. I have triaged them with the enhancement: output format label.

So we must come-up with a general API design for applying options (including set of options) to an *assert.Assertions and *require.Assertions object.