fxtest: Add RequireRun helper #1243
Conversation
|
|
There was a problem hiding this comment.
This feels like it would be easy to misuse.
For most normal applications, RequireRun will block indefinitely—until they try to cancel the test.
The idea of finishing running is true only for applications that use fx.Shutdowner to stop after doing something. Cases like that can currently be tested by something the following:
app := fxtest.New(t, ...)
defer app.RequireStart().RequireStop()
<-app.Wait()
This is equivalent to the proposed RequireRun function and only one extra line.
I'd like us to consider the risk of confusion of the indefinitely blocking RequireRun function versus one extra line with the current available APIs.
(If we were to add RequireRun, it would absolutely have to require a timeout argument so that there was an upper bound on the Wait().)
|
I don't think the misuse issue is that bad here. Yes one could get confused and use |
I'm not sure if I agree with this -- how would it be "clear" that the test is timing out because of RequireRun? |
I added the following code in t.Run("Will timeout", func(t *testing.T) {
t.Parallel()
spy := newTB()
New(
spy,
fx.Invoke(func(lc fx.Lifecycle, s fx.Shutdowner) {}),
).RequireRun() // <- This is misuse of RequireRun on a fx app that doesn't call Shutdown()
})Running the tests gives a stacktrace that points to a |
fxtestprovides both aRequireStart()andRequireStop()helper to test that an fx application can start or stop without errors.But there's currently no way to verify that an fx application can
Run()without error.This PR adds a
RequireRun()method onfxtest.Appthat starts, runs and stops the app, and fails the test if either start or stop failed, or if run returned a non zero exit code