Skip to content

Commit

Permalink
Addressed Sanjay's feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
mwhittaker committed Jun 20, 2023
1 parent 9f4743e commit 118ea2c
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 198 deletions.
4 changes: 2 additions & 2 deletions component.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ type Implements[T any] struct {

// Given a component implementation type, there is currently no nice way,
// using reflection, to get the corresponding component interface type [1].
// The __component_interface_type field exists to make it possible.
// The component_interface_type field exists to make it possible.
//
// [1]: https://github.com/golang/go/issues/54393.
__component_interface_type T //nolint:unused
component_interface_type T //nolint:unused
}

// setInstance is used during component initialization to fill Implements.component.
Expand Down
4 changes: 2 additions & 2 deletions weavertest/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (r Runner) sub(t testing.TB, isBench bool, testBody any) {
t.Fatal(fmt.Errorf("weavertest.Run argument: %v", err))
}

// Assume a component Foo with implementing struct foo. We disallow tests
// Assume a component Foo implementing struct foo. We disallow tests
// like the one below where the user provides a fake and a component
// implementation pointer for the same component.
//
Expand Down Expand Up @@ -281,7 +281,7 @@ func extractComponentInterfaceType(t reflect.Type) (reflect.Type, error) {
return nil, fmt.Errorf("type %v is not a struct", t)
}
// See the definition of weaver.Implements.
f, ok := t.FieldByName("__component_interface_type")
f, ok := t.FieldByName("component_interface_type")
if !ok {
return nil, fmt.Errorf("type %v does not embed weaver.Implements", t)
}
Expand Down
22 changes: 2 additions & 20 deletions weavertest/internal/chain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// Package chain defines a chain of components A -> B -> C -> D with methods to
// Package chain defines a chain of components A -> B -> C with methods to
// propagate a value from the head of the chain to the tail of the chain.
package chain

Expand All @@ -37,10 +37,6 @@ type C interface {
Propagate(context.Context, int) error
}

type D interface {
Propagate(context.Context, int) error
}

type a struct {
weaver.Implements[A]
b weaver.Ref[B]
Expand All @@ -57,13 +53,6 @@ type b struct {

type c struct {
weaver.Implements[C]
d weaver.Ref[D]
mu sync.Mutex
val int
}

type d struct {
weaver.Implements[D]
mu sync.Mutex
val int
}
Expand All @@ -82,16 +71,9 @@ func (b *b) Propagate(ctx context.Context, val int) error {
return b.c.Get().Propagate(ctx, val+1)
}

func (c *c) Propagate(ctx context.Context, val int) error {
func (c *c) Propagate(_ context.Context, val int) error {
c.mu.Lock()
defer c.mu.Unlock()
c.val = val
return c.d.Get().Propagate(ctx, val+1)
}

func (d *d) Propagate(_ context.Context, val int) error {
d.mu.Lock()
defer d.mu.Unlock()
d.val = val
return nil
}
36 changes: 8 additions & 28 deletions weavertest/internal/chain/chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (
func TestOneComponentImpl(t *testing.T) {
// Tests weaver.Test with a component implementation pointer argument.
for _, runner := range weavertest.AllRunners() {
runner.Test(t, func(t *testing.T, a A, d *d) {
runner.Test(t, func(t *testing.T, a A, c *c) {
if err := a.Propagate(context.Background(), 1); err != nil {
t.Fatal(err)
}
if got, want := d.val, 4; got != want {
if got, want := c.val, 3; got != want {
t.Fatalf("got %d, want %d", got, want)
}
})
Expand All @@ -38,11 +38,11 @@ func TestOneComponentImpl(t *testing.T) {
func BenchOneComponentImpl(b *testing.B) {
// Tests weaver.Bench with a component implementation pointer argument.
for _, runner := range weavertest.AllRunners() {
runner.Bench(b, func(b *testing.B, a A, d *d) {
runner.Bench(b, func(b *testing.B, a A, c *c) {
if err := a.Propagate(context.Background(), 1); err != nil {
b.Fatal(err)
}
if got, want := d.val, 4; got != want {
if got, want := c.val, 3; got != want {
b.Fatalf("got %d, want %d", got, want)
}
})
Expand All @@ -52,24 +52,7 @@ func BenchOneComponentImpl(b *testing.B) {
func TestTwoComponentImpls(t *testing.T) {
// Tests weaver.Test with two component implementation pointer arguments.
for _, runner := range weavertest.AllRunners() {
runner.Test(t, func(t *testing.T, a A, c *c, d *d) {
if err := a.Propagate(context.Background(), 1); err != nil {
t.Fatal(err)
}
if got, want := c.val, 3; got != want {
t.Fatalf("got %d, want %d", got, want)
}
if got, want := d.val, 4; got != want {
t.Fatalf("got %d, want %d", got, want)
}
})
}
}

func TestThreeComponentImpls(t *testing.T) {
// Tests weaver.Test with three component implementation pointer arguments.
for _, runner := range weavertest.AllRunners() {
runner.Test(t, func(t *testing.T, a A, b *b, c *c, d *d) {
runner.Test(t, func(t *testing.T, a A, b *b, c *c) {
if err := a.Propagate(context.Background(), 1); err != nil {
t.Fatal(err)
}
Expand All @@ -79,9 +62,6 @@ func TestThreeComponentImpls(t *testing.T) {
if got, want := c.val, 3; got != want {
t.Fatalf("got %d, want %d", got, want)
}
if got, want := d.val, 4; got != want {
t.Fatalf("got %d, want %d", got, want)
}
})
}
}
Expand All @@ -91,14 +71,14 @@ func TestDuplicateComponentImpl(t *testing.T) {
// of type *d. Both pointers should point to the same underlying instance
// of d.
for _, runner := range weavertest.AllRunners() {
runner.Test(t, func(t *testing.T, a A, d1 *d, d2 *d) {
runner.Test(t, func(t *testing.T, a A, c1 *c, c2 *c) {
if err := a.Propagate(context.Background(), 1); err != nil {
t.Fatal(err)
}
if got, want := d1.val, 4; got != want {
if got, want := c1.val, 3; got != want {
t.Fatalf("got %d, want %d", got, want)
}
if got, want := d2.val, 4; got != want {
if got, want := c2.val, 3; got != want {
t.Fatalf("got %d, want %d", got, want)
}
})
Expand Down
146 changes: 0 additions & 146 deletions weavertest/internal/chain/weaver_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 118ea2c

Please sign in to comment.