Skip to content

Commit

Permalink
Don't automatically run Main from weavertests.
Browse files Browse the repository at this point in the history
> Overview

In #359, we changed weavertest to automatically run the main component's
Main method for every test. This was done to allow a developer to test
the server they started in the Main method.

However, we realized that this approach of automatically running the
Main method has some drawbacks. Because the Main method is run for every
test, if you do anything in the Main method that cannot be run more than
once, the tests will fail. For example, if you register a handler with
the default serve mux using `http.HandleFunc`, all tests after the first
will panic.

After some offline discussion, we decided not to run the Main method
automatically. Additionally, we'll allow people to get pointers to
component implementations in a test. Then, developers can test the guts
of their Main method using existing things like the httptest package.
This PR changes weavertest to not run the Main method automatically. In
a future CL, I'll allow developers to get pointers to component
implementations.

> Details

- I removed weavertest.GetListenerAddress. It will no longer be needed.
- I removed the apps variable from the weavertest package. It was only
  needed to implement GetListenerAddress.
- I removed testMainInterface from the weavertest package. I think this
  was some stale code lingering around from an earlier implementation of
  weavertest.
  • Loading branch information
mwhittaker committed Jun 21, 2023
1 parent 004abd4 commit 4c9bcfc
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 178 deletions.
6 changes: 2 additions & 4 deletions examples/chat/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,13 @@ func (db *db) GetImage(ctx context.Context, _ string, image ImageID) ([]byte, er
}

func TestServer(t *testing.T) {
t.Skip("TODO(mwhittaker): Enable testing of main.")
for _, r := range weavertest.AllRunners() {
// Use a fake DB since normal implementation does not work with multiple replicas
db := &db{}
r.Fakes = []weavertest.FakeComponent{weavertest.Fake[SQLStore](db)}
r.Test(t, func(t *testing.T, store weaver.Main) {
addr, err := weavertest.ListenerAddress(t, "chat")
if err != nil {
t.Fatal(err)
}
addr := "TODO(mwhittaker): Enable testing of main"

// Login
expect(t, "login", httpGet(t, addr, "/"), "Login")
Expand Down
2 changes: 0 additions & 2 deletions godeps.txt
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,6 @@ github.com/ServiceWeaver/weaver/weavertest
context
errors
fmt
github.com/ServiceWeaver/weaver
github.com/ServiceWeaver/weaver/internal/envelope/conn
github.com/ServiceWeaver/weaver/internal/private
github.com/ServiceWeaver/weaver/internal/reflection
Expand All @@ -938,7 +937,6 @@ github.com/ServiceWeaver/weaver/weavertest
github.com/ServiceWeaver/weaver/runtime/protos
github.com/google/uuid
go.opentelemetry.io/otel/sdk/trace
go.opentelemetry.io/otel/trace
golang.org/x/exp/maps
golang.org/x/sync/errgroup
os
Expand Down
5 changes: 0 additions & 5 deletions internal/private/private.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,4 @@ type App interface {

// Get fetches the component with type t from wlet.
Get(requester string, t reflect.Type) (any, error)

// ListenerAddress returns the address (host:port) of the
// named listener, waiting for the listener to be created
// if necessary.
ListenerAddress(name string) (string, error)
}
4 changes: 0 additions & 4 deletions weavertest/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ func newDeployer(ctx context.Context, wlet *protos.EnvelopeInfo, config *protos.
log: logWriter,
}

// Force testMain to be local.
d.local["github.com/ServiceWeaver/weaver/weavertest/testMainInterface"] = true

// Fakes need to be local as well.
for _, fake := range runner.Fakes {
name := fmt.Sprintf("%s/%s", fake.intf.PkgPath(), fake.intf.Name())
Expand Down Expand Up @@ -159,7 +156,6 @@ func (d *deployer) start() error {
Sections: d.wlet.Sections,
SingleProcess: d.wlet.SingleProcess,
SingleMachine: d.wlet.SingleMachine,
RunMain: true,
}
bootstrap := runtime.Bootstrap{
ToWeaveletFile: toWeaveletReader,
Expand Down
111 changes: 20 additions & 91 deletions weavertest/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@ package weavertest

import (
"context"
"errors"
"fmt"
"os"
"reflect"
"runtime"
"sync"
"testing"
"time"

"github.com/ServiceWeaver/weaver"
"github.com/ServiceWeaver/weaver/internal/private"
"github.com/ServiceWeaver/weaver/internal/reflection"
"github.com/ServiceWeaver/weaver/runtime/logging"
Expand Down Expand Up @@ -91,58 +88,8 @@ func Fake[T any](impl any) FakeComponent {
return FakeComponent{intf: t, impl: impl}
}

// private.App object per live test or benchmark.
var (
appMu sync.Mutex
apps map[testing.TB]private.App
)

func registerApp(t testing.TB, app private.App) {
appMu.Lock()
defer appMu.Unlock()
if apps == nil {
apps = map[testing.TB]private.App{}
}
apps[t] = app
}

func unregisterApp(t testing.TB, app private.App) {
appMu.Lock()
defer appMu.Unlock()
delete(apps, t)
}

func getApp(t testing.TB) private.App {
appMu.Lock()
defer appMu.Unlock()
return apps[t]
}

// ListenerAddress returns the address (of the form host:port) for the
// listener with the specified name. This call will block waiting for
// the listener to be initialized if necessary.
func ListenerAddress(t testing.TB, name string) (string, error) {
app := getApp(t)
if app == nil {
return "", fmt.Errorf("Service Weaver application is not running")
}
return app.ListenerAddress(name)
}

//go:generate ../cmd/weaver/weaver generate

// testMain is the component implementation used in tests.
type testMain struct {
weaver.Implements[testMainInterface]
}

// testMainInterface is the alternative to weaver.Main we use so that
// we do not conflict with any application provided implementation of
// weaver.Main.
type testMainInterface interface{}

// Test runs a sub-test of t that tests supplied Service Weaver
// application code. It fails at runtime if body is not a function
// Test runs a sub-test of t that tests the supplied Service Weaver
// application code. It fails at runtime if body is not a function
// whose signature looks like:
//
// func(*testing.T, ComponentType1)
Expand All @@ -154,17 +101,20 @@ type testMainInterface interface{}
// followed by a the list of components.
//
// func TestFoo(t *testing.T) {
// weavertest.Local.Test(t, func(t *testing.T, foo Foo, bar Bar) {
// // Test foo and bar ...
// })
// weavertest.Local.Test(t, func(t *testing.T, foo Foo, bar Bar) {
// // Test foo and bar ...
// })
// }
//
// In contrast with weaver.Run, the Test method does not run the Main method of
// any registered weaver.Main component.
func (r Runner) Test(t *testing.T, body any) {
t.Helper()
t.Run(r.Name, func(t *testing.T) { r.sub(t, false, body) })
}

// Bench runs a sub-benchmark of b that benchmarks supplied Service
// Weaver application code. It fails at runtime if body is not a
// Bench runs a sub-benchmark of b that benchmarks the supplied Service
// Weaver application code. It fails at runtime if body is not a
// function whose signature looks like:
//
// func(*testing.B, ComponentType1)
Expand All @@ -176,12 +126,15 @@ func (r Runner) Test(t *testing.T, body any) {
// followed by a the list of components.
//
// func BenchmarkFoo(b *testing.B) {
// weavertest.Local.Bench(b, func(b *testing.B, foo Foo) {
// for i := 0; i < b.N; i++ {
// ... use foo ...
// }
// })
// weavertest.Local.Bench(b, func(b *testing.B, foo Foo) {
// for i := 0; i < b.N; i++ {
// // Benchmark foo ...
// }
// })
// }
//
// In contrast with weaver.Run, the Bench method does not run the Main method of
// any registered weaver.Main component.
func (r Runner) Bench(b *testing.B, testBody any) {
b.Helper()
b.Run(r.Name, func(b *testing.B) { r.sub(b, true, testBody) })
Expand Down Expand Up @@ -260,7 +213,7 @@ func checkRunFunc(t testing.TB, fn any) (func(context.Context, private.App) erro
args[0] = reflect.ValueOf(t)
for i := 1; i < n; i++ {
argType := fnType.In(i)
comp, err := app.Get("weavertest.testMainInterface", argType)
comp, err := app.Get(t.Name(), argType)
if err != nil {
return err
}
Expand All @@ -281,31 +234,7 @@ func runWeaver(ctx context.Context, t testing.TB, runner Runner, body func(conte
if err != nil {
return err
}
registerApp(t, app)
t.Cleanup(func() { unregisterApp(t, app) })

// Run wait() in a go routine.
sub, cancel := context.WithCancel(ctx)
defer cancel()
result := make(chan error)
go func() { result <- app.Wait(sub) }()

// Run the test code.
if err := body(ctx, app); err != nil {
return err
}

// Wait for wait() to finish, but give up after a while in case user Main hangs.
cancel()
select {
case err := <-result:
if err != nil && !errors.Is(err, context.Canceled) {
t.Fatalf("weaver.Main.Main failure: %v", err)
}
case <-time.After(time.Millisecond * 100):
t.Log("weaver.Main.Main not exiting after cancellation")
}
return nil
return body(ctx, app)
}

// logStacks prints the stacks of live goroutines. This functionality
Expand Down
72 changes: 0 additions & 72 deletions weavertest/weaver_gen.go

This file was deleted.

0 comments on commit 4c9bcfc

Please sign in to comment.