Flakey test: TestOnInterruptFunc

[mislav]

What operating system and terminal are you using? any non-Windows platform

An example that showcases the bug.

$ while true; do go test -count=1 -run ^TestOnInterruptFunc$ github.com/AlecAivazis/survey/v2 || break; done

What did you expect to see? test always passes

What did you see instead?

ok      github.com/AlecAivazis/survey/v2    0.085s
ok      github.com/AlecAivazis/survey/v2    0.085s
ok      github.com/AlecAivazis/survey/v2    0.086s
ok      github.com/AlecAivazis/survey/v2    0.102s
ok      github.com/AlecAivazis/survey/v2    0.240s
Ended abruptly!
The end.
--- FAIL: TestOnInterruptFunc (0.00s)
    --- FAIL: TestOnInterruptFunc/Global_OnInterrupt (0.00s)
        survey_test.go:350: Raw output: "? Are you a bot?^C\r\n\x1b[?25l\x1b[?25h? Are you a bot? \r\n"
        survey_test.go:350:
            ? Are you a bot?^C
            ? Are you a bot?
        Error Trace:    survey_test.go:360
        Error:          Not equal:
                        expected: *errors.errorString(&errors.errorString{s:"interrupt"})
                        actual  : <nil>(<nil>)
        Test:           TestOnInterruptFunc/Global_OnInterrupt
FAIL
FAIL    github.com/AlecAivazis/survey/v2    0.463s
FAIL

This is caused by a race condition in a test added in #301 that spawns several t.Run() blocks, which themselves run in separate goroutines that enable parallel test runs by invoking the RunTest() helper, but these tests also depend on the state of the global package-level variable OnInterrupt that changes value during the test.

I could easily fix this test in a PR, but I would argue that https://github.com/AlecAivazis/survey/pull/301 should be reverted instead. /cc @infinitepr0

[infalmo]

You're saying a global level OnInterrupt is a bad idea?

[infalmo]

I don't see any need to revert the PR, since there's no breaking changes. The panic that happened in #319 is because I failed to account for the case when OnInterrupt function is not set, which is fixed by #322. I still don't get how it managed to not panic on test "No OnInterrupt" tho.

Maybe you could help me understand why it didn't FAIL?

[AlecAivazis]

Closing this issue since it no longer applies. Thanks again @mislav