support register teardown function in VerifyTestMain

[tisonkun]

TestMain can be used for setup and teardown logic. However, VerifyTestMain calls os.Exit which disables to register teardown function by defer. cc @prashantv

[abhinav]

Thanks for the report and the PR @tisonkun. We're not certain that that's the best way to accomplish this, especially because VerifyTestMain and VerifyNone already accept functional options.

One solution is a new option that applies only to VerifyNone and VerifyTestMain.

func Cleanup(func()) Option

I don't know if this is the best solution to this problem, but it's a possibility.

Ref internal issue: GO-888

[tisonkun]

@abhinav thanks for your suggestion, I'll think of it. The solution in #65 is just what I implement for pingcap/tidb for the same purpose, it is finished out of the library.

[tisonkun]

GO-888

Is this a ticket number of uber internal issue tracker? I guess it is a golang discussion but fail to find it.

[abhinav]

GO-888

Is this a ticket number of uber internal issue tracker? I guess it is a golang discussion but fail to find it.

Yeah, sorry it's an internal issue tracker link. There's no discussion on it; it's just a tracking task for this issue in our internal issue tracker.

[tisonkun]

@abhinav between these two suggestion, I prefer #65 .

It accepts the return code so that we may skip callbacks or customize logics.

Besides, Option is used already to ignore goroutines (I think most of our users assume this). Overriding the concept and making it a bit different may cause unnecessary misleading. Also highly possibly increase cyclomatic complexity where Option in use.

[tisonkun]

@abhinav today I find a time to attempt the idea you proposed above about a new Option. It can be defined as:

type opts struct {
    filters    []func(stack.Stack) bool
    maxRetries int
    maxSleep   time.Duration
    cleanup    func(int) int
}

func Cleanup(f func(int) int) Option {
    return optionFunc(func(opts *opts) {
        opts.cleanup = f
    })
}

However, I noticed that VerifyNone doesn't need an extra cleanup function as it can be done by defer or any suite's teardown defers. The reason we need a new feature is that: in VerifyTestMain, we call os.Exit which causes defers doesn't have a chance to run. So it's a dedicate wrapper for VerifyTestMain.

So, I still prefer the solution in #65. If we add an option, it only takes effect for VerifyTestMain but not VerifyNone (or we should not), then it looks weird. What do you think?