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

panic is hidden if there are any failed expectations #63

Closed
emilien-puget opened this issue May 19, 2024 · 7 comments · Fixed by #67
Closed

panic is hidden if there are any failed expectations #63

emilien-puget opened this issue May 19, 2024 · 7 comments · Fixed by #67
Assignees

Comments

@emilien-puget
Copy link
Contributor

this unit test shows the issue, the test without the mock reporter will not show the panic and will instead report a fail expectation.

package simple

import (
	"testing"

	. "github.com/ovechkin-dm/mockio/mock"
	"github.com/ovechkin-dm/mockio/tests/common"
)

type myInterface interface {
	Foo(a int) int
}

func TestSimple_panic_mock_reporter(t *testing.T) {
	r := common.NewMockReporter(t)
	SetUp(r)
	m := Mock[myInterface]()
	WhenSingle(m.Foo(Any[int]())).ThenReturn(42).Verify(Once())
	panic("boom")
	m.Foo(10)
}

func TestSimple_panic(t *testing.T) {
	SetUp(t)
	m := Mock[myInterface]()
	WhenSingle(m.Foo(Any[int]())).ThenReturn(42).Verify(Once())
	panic("boom")
	m.Foo(10)
}
@emilien-puget
Copy link
Contributor Author

may be linked to #59

@emilien-puget
Copy link
Contributor Author

func (e *EnrichedReporter) StackTraceErrorf(format string, args ...any) {
	s := NewStackTrace()
	result := fmt.Sprintf(format, args...)
	var st string
	if e.cfg.PrintStackTrace {
		st = fmt.Sprintf(`At:
	%s
Cause:
	%s
Trace:
%s
`, s.CallerLine(), result, s.WithoutLibraryCalls().String())
	} else {
		st = fmt.Sprintf(`At:
	%s
Cause:
	%s
`, s.CallerLine(), result)
	}

	fmt.Println(st)
	// e.reporter.Fatalf(st)
}

changing the last line to a simple print showed me the panic

here is some related issues i found about this topic

golang/go#41355
golang/go@a408139
golang/mock#428

@emilien-puget
Copy link
Contributor Author

switching to ErrorF instead of FatalF seems to solve the issue but this should have been fixed in the go std lib so i am a but confused

// ErrorReporter is an interface for reporting errors during test execution.
// Implementations of this interface should provide a way to fail the test with a message.
type ErrorReporter interface {
	// Fatalf reports an error and fails the test execution.
	// It formats the message according to a format specifier and arguments
	// It can be used to report an error and provide additional context about the error.
	Fatalf(format string, args ...any)
	Errorf(format string, args ...any)
	// Cleanup adds hooks that are used to clean up data after test was executed.
	Cleanup(func())
}

@ovechkin-dm
Copy link
Owner

Thanks for pointing that out.
I don’t have the access to my laptop this week. But from what I can see, the solution could be to not invoke cleanup() verification, if the test is failed already.

@ovechkin-dm ovechkin-dm self-assigned this May 27, 2024
@ovechkin-dm ovechkin-dm linked a pull request May 27, 2024 that will close this issue
@ovechkin-dm
Copy link
Owner

The problem is not that trivial as it turned out.
So as you suggested, I've used Errorf for cleanup and Fatalf for other scenarios, because I still want tests to fail early.
So the output of panicing test will be like this:

=== RUN   TestCleanupShadowsPanic
    reporter.go:67: At:
        	/usr/local/go/src/testing/testing.go:1175 +0x10f
        Cause:
        	expected num method calls: 1, got : 0
        		Foo.Baz(Equal[int], Equal[int], Equal[int])
        
        Trace:
        testing.(*common).Cleanup.func1()
        	/usr/local/go/src/testing/testing.go:1175 +0x10f
        testing.(*common).runCleanup(0xc0001824e0, 0xc000006e00?)
        	/usr/local/go/src/testing/testing.go:1353 +0xdb
        testing.tRunner.func2()
        	/usr/local/go/src/testing/testing.go:1683 +0x25
        panic({0x5f8880?, 0x6937d0?})
        	/usr/local/go/src/runtime/panic.go:770 +0x132
        github.com/ovechkin-dm/mockio/tests/reporting.TestCleanupShadowsPanic(0xc0001824e0?)
        	/home/dovechkin/workspace/mockio/tests/reporting/error_reporting_test.go:203 +0xe6
        testing.tRunner(0xc0001824e0, 0x6506f0)
        	/usr/local/go/src/testing/testing.go:1689 +0xfb
        created by testing.(*T).Run in goroutine 1
        	/usr/local/go/src/testing/testing.go:1742 +0x390
        
--- FAIL: TestCleanupShadowsPanic (0.00s)
panic: failed [recovered]
	panic: failed

goroutine 2 [running]:
testing.tRunner.func1.2({0x5f8880, 0x6937d0})
	/usr/local/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1634 +0x377
panic({0x5f8880?, 0x6937d0?})
	/usr/local/go/src/runtime/panic.go:770 +0x132
github.com/ovechkin-dm/mockio/tests/reporting.TestCleanupShadowsPanic(0xc0001824e0?)
	/home/dovechkin/workspace/mockio/tests/reporting/error_reporting_test.go:203 +0xe6
testing.tRunner(0xc0001824e0, 0x6506f0)
	/usr/local/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
	/usr/local/go/src/testing/testing.go:1742 +0x390


Process finished with the exit code 1

Can you please review PR and maybe add some suggestions?

@emilien-puget
Copy link
Contributor Author

Some tests would be appreciated, as it is quite a weird case

@ovechkin-dm
Copy link
Owner

Closed the issue, fixed in v0.6.2.

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

Successfully merging a pull request may close this issue.

2 participants