Make mockT.Fatal halt execution

[appilon]

The current mockT did not halt execution on Fatal, this led to a test failure as the SDK code expects t.Fatal to halt execution. This introduces a panic and recover to fix the unit test. Future use of the mock (if there are any) should follow the same pattern

[paultyng]

Maybe instead you could add FailNow to the MockT?

[paultyng]

You could alternatively just switch to this package: https://github.com/mitchellh/go-testing-interface

[appilon]

@paultyng We can switch to it in V2. This PR doesn't remove anything it fixes a bug where a mock test continues execution after t.Fatal and the test eventually nil panics (because our code expected it to end at t.Fatal). This PR just adds the return statements to patch that issue.

[appilon]

@paultyng would I be reimplementing FailNow() to actually call runtime.Goexit The issue is our mock doesn't make the called function exit, leading to one test running into nil panic. The return statements in this PR fix the problem also.

[paultyng]

Yeah i guess in this context I'd say fix the mock, not change the code to be incorrect in a normal context. Additionally you could change these returns to panics, as they should not be hit in a normal situation?

[paultyng]

You could simulate it for testing via panic/recover instead of using exit.

[appilon]

@paultyng fixed with recover/panic

[appilon]

Once this is cherry-picked to V2 it will fix a test bug in #357

[paultyng]

just to sanity check here, i would ensure the recover value is "mockT.Fatal" or something and not an unrelated panic.

[kmoe]

Excellent, thank you for chasing this down

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.