Print underlying object in ptr comparison

[mchlp]

Summary

Prints out the underlying value of a pointer to a basic type in addition to the pointer address when the comparison between two basic type pointers fails.

Changes

• Updated truncatingFormat function in assert/assertions.go with a flag that control whether or not the underlying value of basic types are also printed in addition to their address
• Updated assert/assertions_test.go to fix unit tests that were broken and added additional unit test to cover added code

Motivation

As described in Issue #1118, when the comparison between two pointers fails, the underlying value is not always printed. This gives the impression that it is the address of the pointers that are being compared, which is incorrect, since it is actually the underlying values of the pointers that are being compared.

After further research, we discovered that non-basic types such as maps and structs already print the underlying value when the %#v specifier is used for formatting, so their underlying value is already being printed. However, for basic types such as int, bool, and string, the %#v specifier only prints out the pointer address. Our change will ensure that the underlying value of these basic type pointers are printed as well.

For example, this is the original output when comparing two int pointers:

Error:          Not equal: 
                    expected: (*int)(0x14000120df0)
                    actual  : (*int)(0x14000120df8)

And this is the new output when comparing two int pointers:

Error:          Not equal: 
                                expected: (*int)(0x14000120df0) 5
                                actual  : (*int)(0x14000120df8) 0

Similarly, this is the new output when comparing two string and bool pointers:

Error:          Not equal: 
                                expected: (*string)(0x140001132d0) hello
                                actual  : (*string)(0x140001132e0) 

 Error:          Not equal: 
                                expected: (*bool)(0x14000120fa8) true
                                actual  : (*bool)(0x14000120fa9) false

However, we do not print out the underlying value of basic type pointers when a comparison is being made between two different pointer types. This is the same behaviour as before. For example, for a comparison between an int pointer and a string pointer, this is the output:

Error:          Not equal: 
                                expected: *string((*string)(0x140000997c0))
                                actual  : *int((*int)(0x140000a7608))

Related issues

Closes #1118
Closes #1273

[boyan-soubachov]

Thank you for your contribution :)

[boyan-soubachov]

Shouldn't this not be executed if printValueOfBasicTypes == true?

[mchlp]

@boyan-soubachov Would be great if you could take a look at this and merge if it looks good!

[boyan-soubachov]

Please excuse the late review, I have just one question below.

[boyan-soubachov]

Would it not be neater if we just moved the logic for checking whether to print the value into this function rather than passing it in as an argument at every call?

[mchlp]

The logic for checking whether to print the value is already inside the truncatingFormat function (when it loops through the basicTypes list and checks if the type of data matches any of the types in the basicTypes list).

The use of the printValueOfBasicTypes is an override, where if it is set to false, only the pointer address (and not the underlying value) is returned, regardless of whether it is a basic type or not. The use case of this is in the formatUnequalValues function where two pointers of different types are being compared. In this case, we only want the pointer addresses to be printed and not the underlying values (even if the underlying values are basic types). This emphasizes the fact that the pointers are different types (which is why the comparison failed) and that the underlying value of the pointers do not matter for the comparison.

Also updated the comments for the truncatingFormat function to better explain the use of the printValueOfBasicTypes

[mchlp]

@boyan-soubachov Would be great if you could take a look and merge when you get the chance!

[dropdevrahul]

The MR looks good @boyan-soubachov

[boindil]

@boyan-soubachov please have a look

[mchlp]

Rebased on top of what is currently on master and builds are passing on my local branch (https://github.com/mchlp/testify/actions/runs/7200906284), but are failing here.

@dolmen Do you know why this might be happening? The build failures seem to be very inconsistent (they will pass some runs, but will fail some other runs with the exact same commit). The test of 1.20 failed, but the build of 1.20 passed.

[brackendawson]

I'm with @boyan-soubachov here, don't use a boolean argument to hide a new function inside an exising one.

• Put line 561 to 573 in a different function (maybe call it truncatingFormatValues).
• Put line 575 to 578 in another new function (maybe called truncateString).
• Both truncatingFormat and truncatingFormatValues can end with:

  return truncateString(value)

[brackendawson]

I'm not against your basicTypes but this could be written as:

Suggested change

	for _, t := range basicTypes {
	if v.Kind() == t {
	value = fmt.Sprintf("%#v %v", data, v.Interface())
	break
	}
	}
	if isBasicType(v.Kind()) {
	value = fmt.Sprintf("%#v %v", data, v.Interface())
	}

With:

func isBasicType(k reflect.Kind) bool {
	return k == reflect.String || (k >= reflect.Bool && k <= reflect.Complex128)
}

Rather than iterating.