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

Failure log does not reveal pointers #7

Open
palsivertsen opened this issue Dec 6, 2019 · 6 comments
Open

Failure log does not reveal pointers #7

palsivertsen opened this issue Dec 6, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@palsivertsen
Copy link

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
@flyingmutant flyingmutant added the bug Something isn't working label Dec 6, 2019
@flyingmutant
Copy link
Owner

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

@flyingmutant
Copy link
Owner

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 flyingmutant added the enhancement New feature or request label Dec 9, 2019
@flyingmutant
Copy link
Owner

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.

@neetle
Copy link

neetle commented Mar 17, 2020

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

@flyingmutant
Copy link
Owner

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 flyingmutant reopened this Mar 17, 2020
@neetle
Copy link

neetle commented Mar 17, 2020

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.

@flyingmutant flyingmutant removed the bug Something isn't working label Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants