diff --git a/Makefile b/Makefile index 54953fe94..f03be60ed 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ GO_FILES = $(shell \ find . '(' -path '*/.*' -o -path './vendor' -o -path '*/testdata/*' ')' -prune \ -o -name '*.go' -print | cut -b3-) -MODULES = . ./tools ./docs +MODULES = . ./tools ./docs ./internal/e2e # 'make cover' should not run on docs by default. # We run that separately explicitly on a specific platform. diff --git a/app.go b/app.go index d2b8caf17..bc3ed09d7 100644 --- a/app.go +++ b/app.go @@ -574,12 +574,12 @@ func (app *App) Run() { // Historically, we do not os.Exit(0) even though most applications // cede control to Fx with they call app.Run. To avoid a breaking // change, never os.Exit for success. - if code := app.run(app.Done()); code != 0 { + if code := app.run(app.Wait()); code != 0 { app.exit(code) } } -func (app *App) run(done <-chan os.Signal) (exitCode int) { +func (app *App) run(done <-chan ShutdownSignal) (exitCode int) { startCtx, cancel := app.clock.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() @@ -588,7 +588,8 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { } sig := <-done - app.log().LogEvent(&fxevent.Stopping{Signal: sig}) + app.log().LogEvent(&fxevent.Stopping{Signal: sig.Signal}) + exitCode = sig.ExitCode stopCtx, cancel := app.clock.WithTimeout(context.Background(), app.StopTimeout()) defer cancel() @@ -597,7 +598,7 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { return 1 } - return 0 + return exitCode } // Err returns any error encountered during New's initialization. See the diff --git a/app_internal_test.go b/app_internal_test.go index 6e54f8638..a377dd711 100644 --- a/app_internal_test.go +++ b/app_internal_test.go @@ -23,7 +23,6 @@ package fx import ( "errors" "fmt" - "os" "sync" "testing" @@ -42,7 +41,7 @@ func TestAppRun(t *testing.T) { app := New( WithLogger(func() fxevent.Logger { return spy }), ) - done := make(chan os.Signal) + done := make(chan ShutdownSignal) var wg sync.WaitGroup wg.Add(1) @@ -51,7 +50,7 @@ func TestAppRun(t *testing.T) { app.run(done) }() - done <- _sigINT + done <- ShutdownSignal{Signal: _sigINT} wg.Wait() assert.Equal(t, []string{ diff --git a/internal/e2e/README.md b/internal/e2e/README.md new file mode 100644 index 000000000..0c39606d0 --- /dev/null +++ b/internal/e2e/README.md @@ -0,0 +1,6 @@ +This directory holds end-to-end tests for Fx. +Each subdirectory holds a complete Fx application +and a test for it. + +This is marked as a separate Go module to prevent this code from being bundled +with the Fx library and allow for dependencies that don't leak into Fx. diff --git a/internal/e2e/go.mod b/internal/e2e/go.mod new file mode 100644 index 000000000..c771bc0b7 --- /dev/null +++ b/internal/e2e/go.mod @@ -0,0 +1,21 @@ +module go.uber.org/fx/internal/e2e + +go 1.20 + +require ( + github.com/stretchr/testify v1.8.2 + go.uber.org/fx v1.19.2 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + go.uber.org/atomic v1.7.0 // indirect + go.uber.org/dig v1.16.1 // indirect + go.uber.org/multierr v1.6.0 // indirect + go.uber.org/zap v1.23.0 // indirect + golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) + +replace go.uber.org/fx => ../.. diff --git a/internal/e2e/go.sum b/internal/e2e/go.sum new file mode 100644 index 000000000..55ec2f69e --- /dev/null +++ b/internal/e2e/go.sum @@ -0,0 +1,31 @@ +github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/dig v1.16.1 h1:+alNIBsl0qfY0j6epRubp/9obgtrObRAc5aD+6jbWY8= +go.uber.org/dig v1.16.1/go.mod h1:557JTAUZT5bUK0SvCwikmLPPtdQhfvLYtO5tJgQSbnk= +go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= +go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= +go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= +go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= +go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/e2e/shutdowner_run_exitcode/main.go b/internal/e2e/shutdowner_run_exitcode/main.go new file mode 100644 index 000000000..58872c6ce --- /dev/null +++ b/internal/e2e/shutdowner_run_exitcode/main.go @@ -0,0 +1,34 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "go.uber.org/fx" +) + +func main() { + fx.New( + fx.Invoke(func(shutdowner fx.Shutdowner) error { + shutdowner.Shutdown(fx.ExitCode(20)) + return nil + }), + ).Run() +} diff --git a/internal/e2e/shutdowner_run_exitcode/main_test.go b/internal/e2e/shutdowner_run_exitcode/main_test.go new file mode 100644 index 000000000..849be9fe7 --- /dev/null +++ b/internal/e2e/shutdowner_run_exitcode/main_test.go @@ -0,0 +1,72 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/internal/testutil" +) + +// Hijacks the test binary so that the test can run main() as a subprocess +// instead of trying to compile the program and run it directly. +func TestMain(m *testing.M) { + // If the test binary is named "app", then we're running as a subprocess. + // Otherwise, run the tests. + switch filepath.Base(os.Args[0]) { + case "app": + main() + os.Exit(0) + default: + os.Exit(m.Run()) + } +} + +// Verifies that an Fx program running with Run +// exits with the exit code passed to Shutdowner. +// +// Regression test for https://github.com/uber-go/fx/issues/1074. +func TestShutdownExitCode(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + out := testutil.WriteSyncer{T: t} + + // Run the test binary with the name 'app' so that it runs main(). + cmd := exec.Command(exe) + cmd.Args[0] = "app" + cmd.Stdout = &out + cmd.Stderr = &out + + // The program should exit with code 20. + err = cmd.Run() + require.Error(t, err) + + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + + assert.Equal(t, 20, exitErr.ExitCode()) +} diff --git a/internal/e2e/shutdowner_wait_exitcode/main.go b/internal/e2e/shutdowner_wait_exitcode/main.go new file mode 100644 index 000000000..f89bfce50 --- /dev/null +++ b/internal/e2e/shutdowner_wait_exitcode/main.go @@ -0,0 +1,54 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "context" + "log" + "os" + "time" + + "go.uber.org/fx" +) + +func main() { + app := fx.New( + fx.Invoke(func(shutdowner fx.Shutdowner) error { + shutdowner.Shutdown(fx.ExitCode(20)) + return nil + }), + ) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + if err := app.Start(ctx); err != nil { + log.Fatal(err) + } + + sig := <-app.Wait() + + if err := app.Stop(ctx); err != nil { + log.Fatal(err) + } + + os.Exit(sig.ExitCode) +} diff --git a/internal/e2e/shutdowner_wait_exitcode/main_test.go b/internal/e2e/shutdowner_wait_exitcode/main_test.go new file mode 100644 index 000000000..849be9fe7 --- /dev/null +++ b/internal/e2e/shutdowner_wait_exitcode/main_test.go @@ -0,0 +1,72 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/internal/testutil" +) + +// Hijacks the test binary so that the test can run main() as a subprocess +// instead of trying to compile the program and run it directly. +func TestMain(m *testing.M) { + // If the test binary is named "app", then we're running as a subprocess. + // Otherwise, run the tests. + switch filepath.Base(os.Args[0]) { + case "app": + main() + os.Exit(0) + default: + os.Exit(m.Run()) + } +} + +// Verifies that an Fx program running with Run +// exits with the exit code passed to Shutdowner. +// +// Regression test for https://github.com/uber-go/fx/issues/1074. +func TestShutdownExitCode(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + out := testutil.WriteSyncer{T: t} + + // Run the test binary with the name 'app' so that it runs main(). + cmd := exec.Command(exe) + cmd.Args[0] = "app" + cmd.Stdout = &out + cmd.Stderr = &out + + // The program should exit with code 20. + err = cmd.Run() + require.Error(t, err) + + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + + assert.Equal(t, 20, exitErr.ExitCode()) +} diff --git a/shutdown.go b/shutdown.go index aa81e68d3..d1e1b9553 100644 --- a/shutdown.go +++ b/shutdown.go @@ -80,8 +80,6 @@ type shutdowner struct { // Shutdown broadcasts a signal to all of the application's Done channels // and begins the Stop process. Applications can be shut down only after they // have finished starting up. -// In practice this means Shutdowner.Shutdown should not be called from an -// fx.Invoke, but from a fx.Lifecycle.OnStart hook. func (s *shutdowner) Shutdown(opts ...ShutdownOption) error { for _, opt := range opts { opt.apply(s) diff --git a/shutdown_test.go b/shutdown_test.go index 1f322a2cb..73f30f50d 100644 --- a/shutdown_test.go +++ b/shutdown_test.go @@ -128,6 +128,29 @@ func TestShutdown(t *testing.T) { assert.NoError(t, s.Shutdown(fx.ExitCode(2), fx.ShutdownTimeout(time.Second))) }) + + t.Run("from invoke", func(t *testing.T) { + t.Parallel() + + app := fxtest.New( + t, + fx.Invoke(func(s fx.Shutdowner) { + s.Shutdown() + }), + ) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + require.NoError(t, app.Start(ctx), "error starting app") + defer app.Stop(ctx) + select { + case <-ctx.Done(): + assert.Fail(t, "app did not shutdown in time") + case <-app.Wait(): + // success + } + }) } func TestDataRace(t *testing.T) { diff --git a/signal.go b/signal.go index 1593c5de2..441ae8a92 100644 --- a/signal.go +++ b/signal.go @@ -109,7 +109,6 @@ func (recv *signalReceivers) Start(ctx context.Context) { return } - recv.last = nil recv.finished = make(chan struct{}, 1) recv.shutdown = make(chan struct{}, 1) recv.notify(recv.signals, os.Interrupt, _sigINT, _sigTERM)