rapid icon indicating copy to clipboard operation
rapid copied to clipboard

Failure log does not reveal pointers

Open palsivertsen opened this issue 6 years ago • 6 comments

When a failed test output contains pointers, I'd expect to see the pointer value, not the address

Example code:

package exmaple

import (
	"testing"

	"github.com/flyingmutant/rapid"
)

type Example struct {
	ExposeMe *string
}

func TestRapidLogOutputForPointers(t *testing.T) {
	rapid.Check(t, func(t *rapid.T) {
		rapid.Custom(genExample).Draw(t, "example")
		t.Fail()
	})
}
func genExample(t *rapid.T) Example {
	return Example{
		ExposeMe: rapid.Ptr(rapid.String(), false).Draw(t, "example field").(*string),
	}
}

Expected output:

$ go test ./...
--- FAIL: TestRapidLogOutputForPointers (0.00s)
    example_test.go:14: [rapid] failed after 0 tests: (*T).Fail() called
        To reproduce, specify -run="TestRapidLogOutputForPointers" -rapid.seed=1575628109522093485
        Failed test output:
    example_test.go:15: [rapid] draw example: exmaple.Example{ExposeMe:&"some string here"}
FAIL
FAIL	example	0.009s
FAIL

Actual output:

$ go test ./...
--- FAIL: TestRapidLogOutputForPointers (0.00s)
    example_test.go:14: [rapid] failed after 0 tests: (*T).Fail() called
        To reproduce, specify -run="TestRapidLogOutputForPointers" -rapid.seed=1575628109522093485
        Failed test output:
    example_test.go:15: [rapid] draw example: exmaple.Example{ExposeMe:(*string)(0xc000082fb0)}
FAIL
FAIL	example	0.009s
FAIL

palsivertsen avatar Dec 06 '19 10:12 palsivertsen

Thanks a lot for the detailed bug report, will try to fix this today.

flyingmutant avatar Dec 06 '19 10:12 flyingmutant

Hmm, I though I used the wrong verb from fmt but %#v that is used is about as powerful as fmt can go. Perhaps we should join testify/assert and rely on https://github.com/davecgh/go-spew, but I want to think a bit more before adding first third-party dependency to rapid.

flyingmutant avatar Dec 06 '19 22:12 flyingmutant

While this usability issue is important one, I think I want to leave the things as is for now. The reasons for this are:

  • go-spew seems abandoned (last commit almost 1.5 years ago)
  • go-spew requires unsafe by default
  • using go-spew moves rapid a bit away from "library" thing and more towards "framework"

The last point is probably the most important one. Rapid right now is pretty minimalistic in design and implementation, and I want to try to resist feature creep for as long as possible.

flyingmutant avatar Dec 09 '19 21:12 flyingmutant

Out of curiosity:

  • Is there any reason you didn't use the %+v flag instead of the %#v flag, and
  • Would you feel it's "within scope" to add a hook for users to specify how pretty-values are stringified? like a rapid.T.ValueFormatter

neetle avatar Mar 17 '20 00:03 neetle

As for %#v, the rationale was mostly ease of copy-pasting into an editor.

The hook idea sounds interesting! What do you think the interface should look like, and what use cases should it cover?

flyingmutant avatar Mar 17 '20 20:03 flyingmutant

That's an interesting question. Not sure it's one I can safely answer, haha.

Initial gut feel would be:

func WithEngineOptions(... config) Runner
type config func(engine) // <- unexposed for a reason

type Runner interface { 
  Check(t *testing.T, prop func(*T))
  Run(m StateMachine) func(*T)
}

// avoiding a BC break:
var defaultInstance = WithEngineOptions()

var Check = defaultInstance.Check
var Run = defaultInstance.Run

// As for the available options, I'd say the initial one would be the 
func WithValueFormatter(func(v interface{}) string) 

It would likely be a bit of work, but it would also dodge a bit of heartache when it comes to using this library in anger. Using the WithEngineOptions + predefined functions, you should be able to give users hooks into the runtime, but still retain control on how they work internally.

neetle avatar Mar 17 '20 21:03 neetle